Skip to content

Commit

Permalink
chore: revoke using icon styles and overrides directly in components
Browse files Browse the repository at this point in the history
  • Loading branch information
TarunAdobe committed Nov 27, 2024
1 parent ce61b60 commit 14080bb
Show file tree
Hide file tree
Showing 9 changed files with 88 additions and 149 deletions.
56 changes: 20 additions & 36 deletions packages/accordion/src/AccordionItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ governing permissions and limitations under the License.

import {
CSSResultArray,
ElementSize,
html,
PropertyValues,
SizedMixin,
Expand All @@ -20,45 +21,28 @@ import {
import { property } from '@spectrum-web-components/base/src/decorators.js';
import { Focusable } from '@spectrum-web-components/shared/src/focusable.js';
import { when } from '@spectrum-web-components/base/src/directives.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-chevron100.js';
import chevronIconStyles from '@spectrum-web-components/icon/src/spectrum-icon-chevron.css.js';
import chevronIconOverrides from '@spectrum-web-components/icon/src/icon-chevron-overrides.css.js';
import '@spectrum-web-components/icon/sp-icon.js';
import { Chevron100Icon } from '@spectrum-web-components/icons-ui';

import styles from './accordion-item.css.js';

const chevronIcon: Record<string, () => TemplateResult> = {
s: () => html`
<span class="iconContainer">
<sp-icon-chevron100
class="indicator spectrum-UIIcon-ChevronRight75"
slot="icon"
></sp-icon-chevron100>
</span>
`,
m: () => html`
<span class="iconContainer">
<sp-icon-chevron100
class="indicator spectrum-UIIcon-ChevronRight100"
slot="icon"
></sp-icon-chevron100>
</span>
`,
l: () => html`
<span class="iconContainer">
<sp-icon-chevron100
class="indicator spectrum-UIIcon-ChevronRight200"
slot="icon"
></sp-icon-chevron100>
</span>
`,
xl: () => html`
/** Note:
* The current logic deviates from the earlier design of the accordion
* Earlier the s-accordion size would correspond to the --system-ui-icon-chevron-right-75-icon-size=10px;
* Now its pointing to --spectrum-icon-size-s=16px; and similarly for other sizes
* Need to discuss with the design team to understand the correct size to be used
* and update the logic accordingly
*
* Goal here is to avoid using the direct css of the chevron icon and use the sp-icon component instead
*/
const chevronIcon = (size: ElementSize): TemplateResult => {
return html`
<span class="iconContainer">
<sp-icon-chevron100
class="indicator spectrum-UIIcon-ChevronRight300"
slot="icon"
></sp-icon-chevron100>
<sp-icon class="indicator" slot="icon" size=${size}>
${Chevron100Icon()}
</sp-icon>
</span>
`,
`;
};

/**
Expand All @@ -70,7 +54,7 @@ export class AccordionItem extends SizedMixin(Focusable, {
noDefaultSize: true,
}) {
public static override get styles(): CSSResultArray {
return [styles, chevronIconStyles, chevronIconOverrides];
return [styles];
}

@property({ type: Boolean, reflect: true })
Expand Down Expand Up @@ -109,7 +93,7 @@ export class AccordionItem extends SizedMixin(Focusable, {
}

protected renderChevronIcon = (): TemplateResult => {
return chevronIcon[this.size || 'm']();
return chevronIcon(this.size || 'm');
};

protected override render(): TemplateResult {
Expand Down
7 changes: 7 additions & 0 deletions packages/accordion/src/accordion-item.css
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,10 @@ governing permissions and limitations under the License.
var(--spectrum-accordion-item-header-disabled-color)
);
}

sp-icon {
/* I want to decrease the value of --spectrum-icon-size by 6px here to match the
value of icon-size in the snippet from packages/icon/src/icon-**-overrides.css */
block-size: calc(var(--spectrum-icon-size) - 6px);
inline-size: calc(var(--spectrum-icon-size) - 6px);
}
4 changes: 0 additions & 4 deletions packages/accordion/src/accordion.css
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,3 @@ governing permissions and limitations under the License.

@import url('./spectrum-accordion.css');
@import url('./accordion-overrides.css');

:host {
--spectrum-logical-rotation: ;
}
122 changes: 33 additions & 89 deletions packages/checkbox/src/Checkbox.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ governing permissions and limitations under the License.

import {
CSSResultArray,
DefaultElementSize,
ElementSize,
html,
PropertyValues,
SizedMixin,
Expand All @@ -22,87 +22,37 @@ import {
import { property } from '@spectrum-web-components/base/src/decorators.js';
import { CheckboxMixin } from './CheckboxMixin.js';
import checkboxStyles from './checkbox.css.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-checkmark75.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-checkmark100.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-checkmark200.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-checkmark300.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-dash75.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-dash100.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-dash200.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-dash300.js';
import checkmarkSmallStyles from '@spectrum-web-components/icon/src/spectrum-icon-checkmark.css.js';
import checkmarkSmallOverrides from '@spectrum-web-components/icon/src/icon-checkmark-overrides.css.js';
import dashSmallStyles from '@spectrum-web-components/icon/src/spectrum-icon-dash.css.js';
import dashSmallOverrides from '@spectrum-web-components/icon/src/icon-dash-overrides.css.js';

const checkmarkIcon = {
s: () => {
return html`
<sp-icon-checkmark75
id="checkmark"
class="spectrum-Icon spectrum-UIIcon-Checkmark75"
></sp-icon-checkmark75>
`;
},
m: () => {
return html`
<sp-icon-checkmark100
id="checkmark"
class="spectrum-Icon spectrum-UIIcon-Checkmark100"
></sp-icon-checkmark100>
`;
},
l: () => {
return html`
<sp-icon-checkmark200
id="checkmark"
class="spectrum-Icon spectrum-UIIcon-Checkmark200"
></sp-icon-checkmark200>
`;
},
xl: () => {
return html`
<sp-icon-checkmark300
id="checkmark"
class="spectrum-Icon spectrum-UIIcon-Checkmark300"
></sp-icon-checkmark300>
`;
},
import '@spectrum-web-components/icon/sp-icon.js';
import { Checkmark75Icon } from '@spectrum-web-components/icons-ui';
import { Dash50Icon } from '@spectrum-web-components/icons-ui';

/** Note:
* The current logic deviates from the earlier design of the accordion
* Earlier the s-checkbox size would correspond to the --system-ui-icon-checkbox-75-icon-size=10px;
* Now its pointing to --spectrum-icon-size-s=16px; and similarly for other sizes
* Need to discuss with the design team to understand the correct size to be used
* and update the logic accordingly
*
* Goal here is to avoid using the direct css of the checkbox icon and use the sp-icon component instead
*/
const checkmarkIcon = (size: ElementSize): TemplateResult => {
return html`
<sp-icon id="checkmark" class="spectrum-Icon" size=${size}>
${Checkmark75Icon()}
</sp-icon>
`;
};

const dashIcon = {
s: () => {
return html`
<sp-icon-dash75
id="partialCheckmark"
class="spectrum-Icon spectrum-UIIcon-Dash75"
></sp-icon-dash75>
`;
},
m: () => {
return html`
<sp-icon-dash100
id="partialCheckmark"
class="spectrum-Icon spectrum-UIIcon-Dash100"
></sp-icon-dash100>
`;
},
l: () => {
return html`
<sp-icon-dash200
id="partialCheckmark"
class="spectrum-Icon spectrum-UIIcon-Dash200"
></sp-icon-dash200>
`;
},
xl: () => {
return html`
<sp-icon-dash300
id="partialCheckmark"
class="spectrum-Icon spectrum-UIIcon-Dash300"
></sp-icon-dash300>
`;
},
const dashIcon = (size: ElementSize): TemplateResult => {
return html`
<sp-icon
id="partialCheckmark"
class="spectrum-Icon spectrum-UIIcon-Dash75"
size=${size}
>
${Dash50Icon()}
</sp-icon>
`;
};

/**
Expand Down Expand Up @@ -146,13 +96,7 @@ export class Checkbox extends SizedMixin(CheckboxMixin(SpectrumElement), {
}

public static override get styles(): CSSResultArray {
return [
checkboxStyles,
checkmarkSmallStyles,
dashSmallStyles,
checkmarkSmallOverrides,
dashSmallOverrides,
];
return [checkboxStyles];
}

public override click(): void {
Expand All @@ -173,10 +117,10 @@ export class Checkbox extends SizedMixin(CheckboxMixin(SpectrumElement), {
${super.render()}
<span id="box">
${this.checked
? checkmarkIcon[this.size as DefaultElementSize]()
? checkmarkIcon(this.size as ElementSize)
: html``}
${this.indeterminate
? dashIcon[this.size as DefaultElementSize]()
? dashIcon(this.size as ElementSize)
: html``}
</span>
<label id="label" for="input"><slot></slot></label>
Expand Down
7 changes: 7 additions & 0 deletions packages/checkbox/src/checkbox.css
Original file line number Diff line number Diff line change
Expand Up @@ -29,3 +29,10 @@ governing permissions and limitations under the License.
:host(:empty) label {
display: none;
}

sp-icon {
/* I want to decrease the value of --spectrum-icon-size by 6px here to match the
value of icon-size in the snippet from packages/icon/src/icon-**-overrides.css */
block-size: calc(var(--spectrum-icon-size) - 8px);
inline-size: calc(var(--spectrum-icon-size) - 8px);
}
12 changes: 1 addition & 11 deletions packages/menu/src/MenuItem.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,9 @@ import '@spectrum-web-components/icons-ui/icons/sp-icon-checkmark100.js';
import { LikeAnchor } from '@spectrum-web-components/shared/src/like-anchor.js';
import { Focusable } from '@spectrum-web-components/shared/src/focusable.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-chevron100.js';
import chevronStyles from '@spectrum-web-components/icon/src/spectrum-icon-chevron.css.js';
import chevronIconOverrides from '@spectrum-web-components/icon/src/icon-chevron-overrides.css.js';
import { DependencyManagerController } from '@spectrum-web-components/reactive-controllers/src/DependencyManger.js';

import menuItemStyles from './menu-item.css.js';
import checkmarkStyles from '@spectrum-web-components/icon/src/spectrum-icon-checkmark.css.js';
import checkmarkSmallOverrides from '@spectrum-web-components/icon/src/icon-checkmark-overrides.css.js';
import type { Menu } from './Menu.js';
import { MutationController } from '@lit-labs/observers/mutation-controller.js';
import type { Overlay } from '@spectrum-web-components/overlay';
Expand Down Expand Up @@ -97,13 +93,7 @@ export class MenuItem extends LikeAnchor(
ObserveSlotText(ObserveSlotPresence(Focusable, '[slot="icon"]'))
) {
public static override get styles(): CSSResultArray {
return [
menuItemStyles,
checkmarkStyles,
checkmarkSmallOverrides,
chevronStyles,
chevronIconOverrides,
];
return [menuItemStyles];
}

abortControllerSubmenu!: AbortController;
Expand Down
7 changes: 7 additions & 0 deletions packages/menu/src/menu-item.css
Original file line number Diff line number Diff line change
Expand Up @@ -78,3 +78,10 @@ governing permissions and limitations under the License.
:host([no-wrap]) #label {
display: block;
}

sp-icon {
/* I want to decrease the value of --spectrum-icon-size by 6px here to match the
value of icon-size in the snippet from packages/icon/src/icon-**-overrides.css */
block-size: calc(var(--spectrum-icon-size) - 6px);
inline-size: calc(var(--spectrum-icon-size) - 6px);
}
15 changes: 6 additions & 9 deletions packages/textfield/src/Textfield.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,13 +30,11 @@ import {

import { ManageHelpText } from '@spectrum-web-components/help-text/src/manage-help-text.js';
import { Focusable } from '@spectrum-web-components/shared/src/focusable.js';
import '@spectrum-web-components/icons-ui/icons/sp-icon-checkmark100.js';
import '@spectrum-web-components/icons-workflow/icons/sp-icon-alert.js';

import textfieldStyles from './textfield.css.js';
import checkmarkStyles from '@spectrum-web-components/icon/src/spectrum-icon-checkmark.css.js';
import checkmarkSmallOverrides from '@spectrum-web-components/icon/src/icon-checkmark-overrides.css.js';

import '@spectrum-web-components/icon/sp-icon.js';
import { Checkmark100Icon } from '@spectrum-web-components/icons-ui';
const textfieldTypes = ['text', 'url', 'tel', 'email', 'password'] as const;
export type TextfieldType = (typeof textfieldTypes)[number];

Expand All @@ -50,7 +48,7 @@ export class TextfieldBase extends ManageHelpText(
})
) {
public static override get styles(): CSSResultArray {
return [textfieldStyles, checkmarkStyles, checkmarkSmallOverrides];
return [textfieldStyles];
}

@state()
Expand Down Expand Up @@ -280,10 +278,9 @@ export class TextfieldBase extends ManageHelpText(
`;
} else if (this.valid) {
return html`
<sp-icon-checkmark100
id="valid"
class="icon spectrum-UIIcon-Checkmark100"
></sp-icon-checkmark100>
<sp-icon id="valid" class="icon" size=${this.size}>
${Checkmark100Icon({ hidden: true })}
</sp-icon>
`;
}
return nothing;
Expand Down
7 changes: 7 additions & 0 deletions packages/textfield/src/textfield.css
Original file line number Diff line number Diff line change
Expand Up @@ -141,3 +141,10 @@ textarea {
resize: none;
overflow: hidden;
}

sp-icon {
/* I want to decrease the value of --spectrum-icon-size by 6px here to match the
value of icon-size in the snippet from packages/icon/src/icon-**-overrides.css */
block-size: calc(var(--spectrum-icon-size) - 8px);
inline-size: calc(var(--spectrum-icon-size) - 8px);
}

0 comments on commit 14080bb

Please sign in to comment.