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

test: update themed to handle pseudoelements and special underline styling case #9450

Conversation

jcfranco
Copy link
Member

@jcfranco jcfranco commented May 29, 2024

Related Issue: #7180

Summary

✨🧪✨

Examples

"--calcite-menu-item-action-border-color": {
   shadowSelector: `calcite-action::after`,
   targetProp: "backgroundColor",
},
const tokens: ComponentTestTokens = {
  "--calcite-link-underline-color-start": {
    shadowSelector: `a`, // should probably be .anchor or similar class
    targetProp: "backgroundImage",
    },
  };
  themed(html`
    <calcite-link href="#" title="i am a link" icon-start='launch'>with an icon</calcite-link>`, 
    tokens
  );
});

@jcfranco jcfranco marked this pull request as ready for review May 30, 2024 23:04
@jcfranco jcfranco requested a review from a team as a code owner May 30, 2024 23:04
@jcfranco jcfranco added the skip visual snapshots Pull requests that do not need visual regression testing. label May 30, 2024
Copy link
Contributor

@alisonailea alisonailea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we need to add a wait or something else. It seems to work but the element parts aren't always there.

type E2EElementInternal = E2EElement & {
_elmHandle: ElementHandle;
};

return await (element as E2EElementInternal)._elmHandle.evaluate(
(el, targetProp): string => window.getComputedStyle(el).getPropertyValue(targetProp),
(el, targetProp, pseudoElement): string => window.getComputedStyle(el, pseudoElement).getPropertyValue(targetProp),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💪

@@ -115,13 +121,14 @@ export function themed(componentTestSetup: ComponentTestSetup, tokens: Component
styleTargets[selector][1].push(tokenStyle);
}
if (shadowSelector) {
target = shadowSelector ? await page.find(`${selector} >>> ${shadowSelector}`) : target;
const effectiveShadowSelector = shadowSelector.replace(/::.*$/, "");
target.el = await page.find(`${selector} >>> ${effectiveShadowSelector}`);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this with alert, action, and menu-item. Action passed but when I tested alert and menu-item it only passed when I put a debugger on line 125 and then immediately continued to play the test.

const tokens: ComponentTestTokens = {
        "--calcite-alert-dismiss-progress-background-color": {
          shadowSelector: `.${CSS.dismissProgress}::after`,
          targetProp: "backgroundColor",
        },
      };
      themed(`<calcite-alert open auto-close>`, tokens);
const tokens: ComponentTestTokens = {
      "--calcite-menu-item-action-border-color": [
        {
          shadowSelector: "calcite-action::after",
          targetProp: "backgroundColor",
        },
      ],
    };
    themed(`calcite-menu-item`, tokens);
    ```

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This might be a separate issue since all updates in this PR are synchronous or update existing async code.

I noticed the PR was not synced up with epic/7180-component-tokens, which I have taken care of. Would you mind testing again?

Copy link
Contributor

@alisonailea alisonailea left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wooo!! It passes in dev mode! Looks great!

@jcfranco jcfranco merged commit b848cd2 into epic/7180-component-tokens Jun 4, 2024
5 checks passed
@jcfranco jcfranco deleted the jcfranco/update-theme-to-handle-pseudoelements-and-special-underline-case branch June 4, 2024 20:32
jcfranco added a commit that referenced this pull request Jun 4, 2024
✨🧪🧰✨

Test/Assertion failure messages should provide more context now:

```zsh
 FAIL  src/components/action-pad/action-pad.e2e.ts (7.262 s)
  ● calcite-action-pad › theme › default › is themeable

    expect(received).toBe(expected) // Object.is equality

    Expected: "[--calcite-action-pad-trigger-background-color-active:--calcite-action-group-trigger-background-color-active] rgb(0, 191, 255)"
    Received: "[--calcite-action-pad-trigger-background-color-active:--calcite-action-group-trigger-background-color-active] "
```
```zsh
 FAIL  src/components/handle/handle.e2e.ts
  ● calcite-handle › theme › selected › is themeable

    expect(received).toBe(expected) // Object.is equality

    Expected: "[--calcite-handle-background-color-selected:backgroundColor] rgb(0, 191, 255)"
    Received: "[--calcite-handle-background-color-selected:backgroundColor] rgba(0, 0, 0, 0)"
```
```zsh
 FAIL  src/components/action-group/action-group.e2e.ts (5.418 s)
  ● calcite-action-group › theme › is themeable

    [--calcite-action-group-trigger-background-color-hover] target node (calcite-action-group >>> calcite-action) must be clickable (larger than 1x1) for state: hover

      380 |             })`;
```

**Note**: this depends on
#9450.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip visual snapshots Pull requests that do not need visual regression testing.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants