Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature/add shadow dom test #625

Closed
wants to merge 2 commits into from
Closed

Conversation

diba1013
Copy link

@diba1013 diba1013 commented Jul 21, 2020

Before submission, please check that ...

  • the code follows the Clean Code guidelines.
  • the necessary tests are either created or updated.
  • all status checks (GitHub Actions, SonarCloud, etc.) pass.
  • your commit history is cleaned up.
  • you updated the changelog. (not necessary)
  • you updated necessary documentation within retest/docs. (pending)

Description

TL;DR This adds a test for the current state on shadow DOMs.

The example is custom build, using guides from google developers. It features open, closed and nested shadow elements through the use of custom elements. Furthermore, it makes use of both true shadow DOM and light DOM.

  1. The included logo is an optimized logo from our website, embedded as a symbol into the svg, so that it can be styled and referenced from outside. This creates a closed shadow DOM.
  2. A layout using custom elements is created, which build themselves using custom attributes that are synced to the shadow DOM, automatically updating it on changes.
  3. Slots are used to inject light DOM into the custom components, which are accessible from the outside.

The differences are introduced with a click of a button. These are designed to introduce changes to all aspects of the shadow DOM. It modifies the attributes of custom elements, light elements and through this, indirectly has effect on the shadow DOM (because of data binding).

This test does not use templates for rendering the shadow DOM, because templates might be a separate thing that we need to consider at some time in the future.

All in all, this test should suffice for most scenarios when working with shadow DOMs. Of course, almost everything is possible with the web, and we will see, if it does.

The test uses its own temp directory, because we are only interested in the actual changes, as we have no use for the generated Golden Masters right now. Secondly, it references its own filter instead of the global recheck.ignore to the test report all differences as expected, without changes in the recheck.ignore having an side-effect on this test.

To have a more representable example, this uses some minor bootstrap CSS, mostly the reboot and button elements. As this introduces an external dependency, I am not sure how this might affect the elements. However, we could decide to include this as a local stylesheet.

State of shadow DOM

As the introduced test shows, we currently only gather changes on the custom elements as well as the light DOM. The actual changes that happen in the shadow DOM are currently not tracked.

Based on small initial tests, it should be fairly easy™ to implement a working solution that works for open shadow roots. However, we will potentially need to discuss how we want to handle light DOM, as, although it is physically moved inside the shadow DOM, it is still accessible from the outside.

State of this PR

Due to changes that introduced pseudo-elements #623, the test does not currently have the expected result. This is only the case for the "change" button, which gets disabled once clicked. However, the Alignment gets confused by this and lists it as inserted-deleted element. Right now, the test shows the expected output, which differ from the actual output because of this. This can be addressed the following ways:

  1. Have the test assert the wrong message, so that it will pass.
  2. Wait until merging, until Fix tests that broke after introducing pseudo elements #624 is implemented.

My opinion: The test already shows that the basics are working and the bug is outside the scope of this PR. However, it would introduce a dirty commit history (i.e. a commit that has to be reverted) and since this PR is not critical, we could just leave it as a draft until scenario 2 is done.

I will add documentation so that it is clear to what extend this is working right now, as I don't see that we will implement a proper working solution for this release.

Additional Context

I would like to add this to our assets website as an example, so that it can be referenced throughout our documentation and other articles. This would allow us to remove the website from this repo and only reference it in the test.

This does not work with IE11.

@diba1013 diba1013 added the enhancement New feature or request label Jul 21, 2020
@diba1013 diba1013 self-assigned this Jul 21, 2020
@cla-bot cla-bot bot added the cla-signed label Jul 21, 2020
@diba1013 diba1013 mentioned this pull request Aug 26, 2020
6 tasks
@cla-bot
Copy link

cla-bot bot commented Jan 18, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot cla-bot bot removed the cla-signed label Jan 18, 2022
@cla-bot
Copy link

cla-bot bot commented Jan 18, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Jan 18, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Jan 18, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@cla-bot
Copy link

cla-bot bot commented Jan 19, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

diba1013 added 2 commits January 19, 2022 00:31
which currently is only able to detect light dom and attribute changes
@cla-bot
Copy link

cla-bot bot commented Jan 19, 2022

Thank you for your pull request and welcome to our community. We could not parse the GitHub identity of the following contributors: diba1013.
This is most likely caused by a git client misconfiguration; please make sure to:

  1. check if your git client is configured with an email to sign commits git config --list | grep email
  2. If not, set it up using git config --global user.email [email protected]
  3. Make sure that the git commit email is configured in your GitHub account settings, see https://github.com/settings/emails

@diba1013 diba1013 closed this Jan 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Development

Successfully merging this pull request may close these issues.

1 participant