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.

Sign up for GitHub

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

Generalize mocking of states in dummy pages #314

Merged
merged 6 commits into from
May 26, 2022
Merged

Generalize mocking of states in dummy pages #314

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6cef50b
added controller for the “components” pages that takes care of handli…
didoo May 25, 2022
479aea4
updated `Dropdown` + `Button` dummy pages (and related components and…
didoo May 25, 2022
5420f94
linting
didoo May 25, 2022
2ed672e
generalized `replaceMockStates` function to handle multiple states + …
didoo May 25, 2022
ddfc49a
changes suggested by Jamie White to fix issue with `constructor()`
didoo May 25, 2022
a3ad600
updated tests for CTA to avoid Percy failing tests
didoo May 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/components/addon/components/hds/dropdown/list-item/copy-item.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
{{/if}}
<button
type="button"
class="{{if @state (concat 'is-' @state)}} {{if this.isSuccess 'is-success'}}"
class="{{if this.isSuccess 'is-success'}}"
{{on "click" this.copyCode}}
>
<div class="hds-dropdown-list-item__copy-item-text hds-typography-code-100">
Expand Down
5 changes: 2 additions & 3 deletions packages/components/addon/components/hds/dropdown/list-item/interactive.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
<li class={{this.classNames}}>
{{#if @route}}
<LinkTo
class="{{if @state (concat 'is-' @state)}}"
@current-when={{@current-when}}
@models={{hds-link-to-models @model @models}}
@query={{hds-link-to-query @query}}
Expand All @@ -19,7 +18,7 @@
</div>
</LinkTo>
{{else if @href}}
<a target="_blank" rel="noopener noreferrer" href={{@href}} class="{{if @state (concat 'is-' @state)}}">
<a target="_blank" rel="noopener noreferrer" href={{@href}}>
{{#if @icon}}
<div class="hds-dropdown-list-item__interactive-icon">
<FlightIcon @name={{@icon}} @isInlineBlock={{false}} />
Expand All @@ -30,7 +29,7 @@
</div>
</a>
{{else}}
<button class="{{if @state (concat 'is-' @state)}}" type="button" ...attributes>
<button type="button" ...attributes>
{{#if @icon}}
<div class="hds-dropdown-list-item__interactive-icon">
<FlightIcon @name={{@icon}} @isInlineBlock={{false}} />
Expand Down
38 changes: 19 additions & 19 deletions packages/components/app/styles/components/button.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ $hds-button-focus-border-width: 3px;
// This covers all of the browsers and focus scenarios (due to the custom focus design).
&:disabled,
&[disabled],
&.is-disabled,
&.mock-disabled,
Copy link
Contributor Author

@didoo didoo May 25, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

in the future, this class selectors could be removed using a css post-processor

&:disabled:focus,
&[disabled]:focus,
&.is-disabled:focus,
&.mock-disabled:focus,
&:disabled:hover,
&[disabled]:hover,
&.is-disabled:hover {
&.mock-disabled:hover {
background-color: var(--token-color-surface-faint);
border-color: var(--token-color-border-primary);
box-shadow: none;
Expand All @@ -57,7 +57,7 @@ $hds-button-focus-border-width: 3px;
}

&:focus,
&.is-focus {
&.mock-focus {
box-shadow: none;

&::before {
Expand Down Expand Up @@ -145,15 +145,15 @@ $size-props: (
color: var(--token-color-foreground-high-contrast);

&:hover,
&.is-hover {
&.mock-hover {
background-color: var(--token-color-palette-blue-300);
border-color: var(--token-color-palette-blue-400);
color: var(--token-color-foreground-high-contrast);
cursor: pointer;
}

&:focus,
&.is-focus {
&.mock-focus {
background-color: var(--token-color-palette-blue-200);
border-color: var(--token-color-focus-action-internal);
color: var(--token-color-foreground-high-contrast);
Expand All @@ -172,7 +172,7 @@ $size-props: (
}

&:active,
&.is-active {
&.mock-active {
background-color: var(--token-color-palette-blue-400);
border-color: var(--token-color-palette-blue-400);
box-shadow: none;
Expand All @@ -190,15 +190,15 @@ $size-props: (
color: var(--token-color-foreground-primary);

&:hover,
&.is-hover {
&.mock-hover {
background-color: var(--token-color-surface-primary);
border-color: var(--token-color-border-strong);
color: var(--token-color-foreground-primary);
cursor: pointer;
}

&:focus,
&.is-focus {
&.mock-focus {
background-color: var(--token-color-surface-faint);
border-color: var(--token-color-focus-action-internal);
color: var(--token-color-foreground-primary);
Expand All @@ -208,7 +208,7 @@ $size-props: (
}

&:active,
&.is-active {
&.mock-active {
background-color: var(--token-color-surface-interactive-active);
border-color: var(--token-color-border-strong);
box-shadow: none;
Expand All @@ -225,15 +225,15 @@ $size-props: (
color: var(--token-color-foreground-action);

&:hover,
&.is-hover {
&.mock-hover {
background-color: var(--token-color-surface-primary);
border-color: var(--token-color-border-strong);
color: var(--token-color-foreground-action-hover);
cursor: pointer;
}

&:focus,
&.is-focus {
&.mock-focus {
border-color: var(--token-color-focus-action-internal);
color: var(--token-color-foreground-action);
&::before {
Expand All @@ -242,7 +242,7 @@ $size-props: (
}

&:active,
&.is-active {
&.mock-active {
background-color: var(--token-color-surface-interactive-active);
border-color: var(--token-color-border-strong);
box-shadow: none;
Expand All @@ -254,13 +254,13 @@ $size-props: (

&:disabled,
&[disabled],
&.is-disabled,
&.mock-disabled,
&:disabled:focus,
&[disabled]:focus,
&.is-disabled:focus,
&.mock-disabled:focus,
&:disabled:hover,
&[disabled]:hover,
&.is-disabled:hover {
&.mock-disabled:hover {
background-color: transparent;
border-color: transparent;

Expand All @@ -277,15 +277,15 @@ $size-props: (
color: var(--token-color-foreground-critical-on-surface);

&:hover,
&.is-hover {
&.mock-hover {
background-color: var(--token-color-palette-red-300);
border-color: var(--token-color-palette-red-400);
color: var(--token-color-foreground-high-contrast);
cursor: pointer;
}

&:focus,
&.is-focus {
&.mock-focus {
background-color: var(--token-color-surface-critical);
border-color: var(--token-color-focus-critical-internal);
color: var(--token-color-foreground-critical-on-surface);
Expand All @@ -295,7 +295,7 @@ $size-props: (
}

&:active,
&.is-active {
&.mock-active {
background-color: var(--token-color-palette-red-400);
border-color: var(--token-color-palette-red-400);
box-shadow: none;
Expand Down
20 changes: 10 additions & 10 deletions packages/components/app/styles/components/dropdown.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ $hds-dropdown-toggle-border-radius: 5px;
padding: 1px;

&:hover,
&.is-hover {
&.mock-hover {
background-color: var(--token-color-surface-interactive);
border-color: var(--token-color-border-strong);
cursor: pointer;
Expand All @@ -49,7 +49,7 @@ $hds-dropdown-toggle-border-radius: 5px;
@include hds-focus-ring-with-pseudo-element($top: -1px, $right: -1px, $bottom: -1px, $left: -1px, $radius: $hds-dropdown-toggle-border-radius);

&:active,
&.is-active {
&.mock-active {
background-color: var(--token-color-surface-interactive-active);
border-color: var(--token-color-border-strong);
}
Expand Down Expand Up @@ -166,26 +166,26 @@ $hds-dropdown-toggle-border-radius: 5px;
width: 100%;

&:hover,
&.is-hover {
&.mock-hover {
background-color: var(--token-color-surface-interactive-hover);
cursor: pointer;
}

@include hds-focus-ring-basic();

&:focus,
&.is-focus {
&.mock-focus {
//TODO this focus is just way too complex
background-color: var(--token-color-surface-action);
border-color: var(--token-color-focus-action-internal);
}

&:active,
&.is-active {
&.mock-active {
background-color: var(--token-color-surface-interactive-active);
}

&.is-success {
&.mock-success {
background-color: var(--token-color-surface-success);
border-color: var(--token-color-border-success);

Expand Down Expand Up @@ -275,7 +275,7 @@ $hds-dropdown-toggle-border-radius: 5px;
// and define their values in the color variants below

&:hover,
&.is-hover {
&.mock-hover {
color: var(--current-color-hover);
&::before {
background-color: currentColor;
Expand All @@ -284,7 +284,7 @@ $hds-dropdown-toggle-border-radius: 5px;

// default focus for browsers that still rely on ":focus"
&:focus,
&.is-focus {
&.mock-focus {
color: var(--current-color-focus);
&::after {
background-color: var(--current-background-color);
Expand Down Expand Up @@ -312,7 +312,7 @@ $hds-dropdown-toggle-border-radius: 5px;
// remove the focus ring on "active + focused" state (by design)
&:focus:active,
&:focus-visible:active,
&.is-focus.is-active {
&.mock-focus.mock-active {
&::after {
background-color: var(--current-background-color);
box-shadow: none;
Expand All @@ -321,7 +321,7 @@ $hds-dropdown-toggle-border-radius: 5px;
}

&:active,
&.is-active {
&.mock-active {
color: var(--current-color-active);
&::before {
background-color: currentColor;
Expand Down
12 changes: 8 additions & 4 deletions packages/components/app/styles/mixins/_focus-ring.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,8 @@

// default focus for browsers that still rely on ":focus"
&:focus,
&.is-focus {
&.is-focus,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

once the changes done across all the pages, I'll remove the is-[state] selectors from here, for now we need both to avoid breaking the percy tests

&.mock-focus {
box-shadow: var(--token-focus-ring-#{$color}-box-shadow);
}
// undo the previous declaration for browsers that support ":focus-visible" but wouldn't normally show default focus styles
Expand All @@ -21,7 +22,8 @@
}
// remove the focus ring on "active + focused" state (by design)
&:focus:active,
&.is-focus.is-active {
&.is-focus.is-active,
&.mock-focus.is-active {
box-shadow: none;
}
}
Expand All @@ -45,7 +47,8 @@

// default focus for browsers that still rely on ":focus"
&:focus,
&.is-focus {
&.is-focus,
&.mock-focus {
&::before {
box-shadow: var(--token-focus-ring-#{$color}-box-shadow);
}
Expand All @@ -64,7 +67,8 @@
}
// remove the focus ring on "active + focused" state (by design)
&:focus:active,
&.is-focus.is-active {
&.is-focus.is-active,
&.mock-focus.is-active {
&::before {
box-shadow: none;
}
Expand Down
24 changes: 24 additions & 0 deletions packages/components/tests/dummy/app/controllers/components.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import Controller from '@ember/controller';
import { scheduleOnce } from '@ember/runloop';

function replaceDummyStates() {
document.querySelectorAll('[mock-state-value]').forEach(function (element) {
let target;
if (element.attributes['mock-state-selector']) {
target = element.querySelector(
element.attributes['mock-state-selector'].value
);
} else {
target = element;
}
const state = element.attributes['mock-state-value'].value;
target.classList.add(`mock-${state}`);
});
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The logic here is this: we have two possible custom HTML attributes (not prefixed with data intentionally, even if it's not valid HTML for our purpose is OK)

  • mock-state-value is the value of the state that we want to use, to apply a class mock-[state] to an element, so that a custom CSS class can be used to emulate a particular state in a component (typically pseudo states like :hover or :focus) in a showcase
  • mock-state-selector is the DOM selectors we want to use (relative to the element to which is applied) to select the target element on which dynamically apply the mock class at runtime; if no selector is defined, we simply use the element with the mock-state-value attribute itself

}

export default class ComponentsController extends Controller {
constructor() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

this all feels a bit magical to me but I also don't have a more idiomatic suggestion springing to mind. I think this is ok for "scrappy" but something we should at least be remembering if we run into unforeseen issues in the future.

super(...arguments);
scheduleOnce('afterRender', this, replaceDummyStates);
}
}
10 changes: 8 additions & 2 deletions packages/components/tests/dummy/app/templates/components/button.hbs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,13 @@
<div>
<span class="dummy-text-small">{{capitalize size}}/{{state}}:</span>
<br />
<Hds::Button @icon="plus" @text={{capitalize state}} @size={{size}} @color={{color}} class="is-{{state}}" />
<Hds::Button
@icon="plus"
@text={{capitalize state}}
@size={{size}}
@color={{color}}
mock-state-value={{state}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

here you can see how one of these mock states is declared on a component, but only in the dummy documentation page

/>
</div>
{{/each}}
{{/each}}
Expand All @@ -358,7 +364,7 @@
@text={{capitalize state}}
@color={{color}}
@isFullWidth={{true}}
class="is-{{state}}"
mock-state-value={{state}}
/>
</div>
{{/each}}
Expand Down
Loading