feat: deprecate "icon-right" in buttons as per Spectrum

adobe · Jan 19, 2021 · 064a775 · 064a775
1 parent c3a6420
commit 064a775
Show file tree

Hide file tree

Showing 10 changed files with 5 additions and 98 deletions.
diff --git a/.circleci/config.yml b/.circleci/config.yml
@@ -29,7 +29,7 @@ commands:
                   keys:
                       - v2-golden-images-<< pipeline.parameters.current_golden_images_hash >>-<< parameters.regression_color >>-<< parameters.regression_scale >>-<< parameters.regression_dir >>-
                       - v2-golden-images-main-<< parameters.regression_color >>-<< parameters.regression_scale >>-<< parameters.regression_dir >>-
-            - run: yarn test:visual:ci --concurrency=5 --color=<< parameters.regression_color >> --scale=<< parameters.regression_scale >> --dir=<< parameters.regression_dir >>
+            - run: yarn test:visual:ci --concurrency=4 --color=<< parameters.regression_color >> --scale=<< parameters.regression_scale >> --dir=<< parameters.regression_dir >>
             - run:
                   when: on_fail
                   command: cp -RT test/visual/screenshots-current/ci test/visual/screenshots-baseline/ci

diff --git a/packages/button/src/ButtonBase.ts b/packages/button/src/ButtonBase.ts
@@ -43,9 +43,6 @@ export class ButtonBase extends LikeAnchor(
     @property({ type: String })
     public type: 'button' | 'submit' | 'reset' = 'button';
 
-    @property({ type: Boolean, reflect: true, attribute: 'icon-right' })
-    protected iconRight = false;
-
     protected get hasLabel(): boolean {
         return this.slotHasContent;
     }
@@ -58,9 +55,6 @@ export class ButtonBase extends LikeAnchor(
     }
 
     protected get buttonContent(): TemplateResult[] {
-        const icon = html`
-            <slot name="icon" ?icon-only=${!this.hasLabel}></slot>
-        `;
         const content = [
             html`
                 <div id="label" ?hidden=${!this.hasLabel}>
@@ -71,10 +65,11 @@ export class ButtonBase extends LikeAnchor(
                 </div>
             `,
         ];
-        if (!this.hasIcon) {
-            return content;
+        if (this.hasIcon) {
+            content.unshift(html`
+                <slot name="icon" ?icon-only=${!this.hasLabel}></slot>
+            `);
         }
-        this.iconRight ? content.push(icon) : content.unshift(icon);
         return content;
     }
 

diff --git a/packages/button/src/button.css b/packages/button/src/button.css
@@ -12,14 +12,6 @@ governing permissions and limitations under the License.
 
 @import './spectrum-button.css';
 
-:host([dir='ltr']) #label + slot[name='icon']::slotted(*) {
-    padding-left: var(--spectrum-button-primary-icon-gap);
-}
-
-:host([dir='rtl']) #label + slot[name='icon']::slotted(*) {
-    padding-right: var(--spectrum-button-primary-icon-gap);
-}
-
 :host([size='s']) {
     --spectrum-icon-tshirt-size-height: var(
         --spectrum-alias-workflow-icon-size-s

diff --git a/packages/button/stories/button-cta.stories.ts b/packages/button/stories/button-cta.stories.ts
@@ -9,7 +9,6 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA
 OF ANY KIND, either express or implied. See the License for the specific language
 governing permissions and limitations under the License.
 */
-import { boolean } from '@open-wc/demoing-storybook';
 import { html, TemplateResult } from '@spectrum-web-components/base';
 import { renderButtonSet, bellIcon } from './index.js';
 import { HelpIcon } from '@spectrum-web-components/icons-workflow';
@@ -28,7 +27,6 @@ export const quiet = (): TemplateResult =>
     });
 
 export const withIcon = (): TemplateResult => {
-    const iconRight = boolean('Icon on Right', false);
     return html`
         <style>
             .row {
@@ -38,7 +36,6 @@ export const withIcon = (): TemplateResult => {
         <div class="row">
             ${renderButtonSet({
                 variant,
-                iconRight,
                 content: html`
                     <sp-icon slot="icon">
                         ${HelpIcon({ hidden: true })}
@@ -50,20 +47,11 @@ export const withIcon = (): TemplateResult => {
         <div class="row">
             ${renderButtonSet({
                 variant,
-                iconRight,
                 content: html`
                     ${bellIcon} Custom SVG
                 `,
             })}
         </div>
-        <div class="row">
-            <sp-button variant=${variant} icon-right>
-                ${bellIcon} Custom SVG
-            </sp-button>
-            <sp-button variant=${variant}>
-                ${bellIcon} Custom SVG
-            </sp-button>
-        </div>
     `;
 };
 

diff --git a/packages/button/stories/button-overBackground.stories.ts b/packages/button/stories/button-overBackground.stories.ts
@@ -9,7 +9,6 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA
 OF ANY KIND, either express or implied. See the License for the specific language
 governing permissions and limitations under the License.
 */
-import { boolean } from '@open-wc/demoing-storybook';
 import { html, TemplateResult } from '@spectrum-web-components/base';
 import { renderButtonSet, bellIcon, makeOverBackground } from './index.js';
 import { HelpIcon } from '@spectrum-web-components/icons-workflow';
@@ -29,7 +28,6 @@ export const quiet = (): TemplateResult =>
     });
 
 export const withIcon = (): TemplateResult => {
-    const iconRight = boolean('Icon on Right', false);
     return html`
         <style>
             .row {
@@ -39,7 +37,6 @@ export const withIcon = (): TemplateResult => {
         <div class="row">
             ${renderButtonSet({
                 variant,
-                iconRight,
                 content: html`
                     <sp-icon slot="icon">
                         ${HelpIcon({ hidden: true })}
@@ -51,20 +48,11 @@ export const withIcon = (): TemplateResult => {
         <div class="row">
             ${renderButtonSet({
                 variant,
-                iconRight,
                 content: html`
                     ${bellIcon} Custom SVG
                 `,
             })}
         </div>
-        <div class="row">
-            <sp-button variant=${variant} icon-right>
-                ${bellIcon} Custom SVG
-            </sp-button>
-            <sp-button variant=${variant}>
-                ${bellIcon} Custom SVG
-            </sp-button>
-        </div>
     `;
 };
 

diff --git a/packages/button/stories/button-primary.stories.ts b/packages/button/stories/button-primary.stories.ts
@@ -9,7 +9,6 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA
 OF ANY KIND, either express or implied. See the License for the specific language
 governing permissions and limitations under the License.
 */
-import { boolean } from '@open-wc/demoing-storybook';
 import { html, TemplateResult } from '@spectrum-web-components/base';
 import { renderButtonSet, bellIcon } from './index.js';
 import { HelpIcon } from '@spectrum-web-components/icons-workflow';
@@ -28,7 +27,6 @@ export const quiet = (): TemplateResult =>
     });
 
 export const withIcon = (): TemplateResult => {
-    const iconRight = boolean('Icon on Right', false);
     return html`
         <style>
             .row {
@@ -38,7 +36,6 @@ export const withIcon = (): TemplateResult => {
         <div class="row">
             ${renderButtonSet({
                 variant,
-                iconRight,
                 content: html`
                     <sp-icon slot="icon">
                         ${HelpIcon({ hidden: true })}
@@ -50,20 +47,11 @@ export const withIcon = (): TemplateResult => {
         <div class="row">
             ${renderButtonSet({
                 variant,
-                iconRight,
                 content: html`
                     ${bellIcon} Custom SVG
                 `,
             })}
         </div>
-        <div class="row">
-            <sp-button variant=${variant} icon-right>
-                ${bellIcon} Custom SVG
-            </sp-button>
-            <sp-button variant=${variant}>
-                ${bellIcon} Custom SVG
-            </sp-button>
-        </div>
     `;
 };
 

diff --git a/packages/button/stories/button-secondary.stories.ts b/packages/button/stories/button-secondary.stories.ts
@@ -9,7 +9,6 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA
 OF ANY KIND, either express or implied. See the License for the specific language
 governing permissions and limitations under the License.
 */
-import { boolean } from '@open-wc/demoing-storybook';
 import { html, TemplateResult } from '@spectrum-web-components/base';
 import { renderButtonSet, bellIcon } from './index.js';
 import { HelpIcon } from '@spectrum-web-components/icons-workflow';
@@ -28,7 +27,6 @@ export const quiet = (): TemplateResult =>
     });
 
 export const withIcon = (): TemplateResult => {
-    const iconRight = boolean('Icon on Right', false);
     return html`
         <style>
             .row {
@@ -38,7 +36,6 @@ export const withIcon = (): TemplateResult => {
         <div class="row">
             ${renderButtonSet({
                 variant,
-                iconRight,
                 content: html`
                     <sp-icon slot="icon">
                         ${HelpIcon({ hidden: true })}
@@ -50,20 +47,11 @@ export const withIcon = (): TemplateResult => {
         <div class="row">
             ${renderButtonSet({
                 variant,
-                iconRight,
                 content: html`
                     ${bellIcon} Custom SVG
                 `,
             })}
         </div>
-        <div class="row">
-            <sp-button variant=${variant} icon-right>
-                ${bellIcon} Custom SVG
-            </sp-button>
-            <sp-button variant=${variant}>
-                ${bellIcon} Custom SVG
-            </sp-button>
-        </div>
     `;
 };
 

diff --git a/packages/button/stories/button-warning.stories.ts b/packages/button/stories/button-warning.stories.ts
@@ -9,7 +9,6 @@ the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR REPRESENTA
 OF ANY KIND, either express or implied. See the License for the specific language
 governing permissions and limitations under the License.
 */
-import { boolean } from '@open-wc/demoing-storybook';
 import { html, TemplateResult } from '@spectrum-web-components/base';
 import { renderButtonSet, bellIcon } from './index.js';
 import { HelpIcon } from '@spectrum-web-components/icons-workflow';
@@ -28,7 +27,6 @@ export const quiet = (): TemplateResult =>
     });
 
 export const withIcon = (): TemplateResult => {
-    const iconRight = boolean('Icon on Right', false);
     return html`
         <style>
             .row {
@@ -38,7 +36,6 @@ export const withIcon = (): TemplateResult => {
         <div class="row">
             ${renderButtonSet({
                 variant,
-                iconRight,
                 content: html`
                     <sp-icon slot="icon">
                         ${HelpIcon({ hidden: true })}
@@ -50,20 +47,11 @@ export const withIcon = (): TemplateResult => {
         <div class="row">
             ${renderButtonSet({
                 variant,
-                iconRight,
                 content: html`
                     ${bellIcon} Custom SVG
                 `,
             })}
         </div>
-        <div class="row">
-            <sp-button variant=${variant} icon-right>
-                ${bellIcon} Custom SVG
-            </sp-button>
-            <sp-button variant=${variant}>
-                ${bellIcon} Custom SVG
-            </sp-button>
-        </div>
     `;
 };
 

diff --git a/packages/button/stories/index.ts b/packages/button/stories/index.ts
@@ -22,7 +22,6 @@ interface Properties {
     quiet?: boolean;
     content?: TemplateResult;
     disabled?: boolean;
-    iconRight?: boolean;
     size?: 's' | 'm' | 'l' | 'xl';
     href?: string;
     target?: '_blank' | '_parent' | '_self' | '_top';
@@ -45,7 +44,6 @@ export function renderButton(properties: Properties): TemplateResult {
                 variant="${properties.variant}"
                 ?quiet="${!!properties.quiet}"
                 ?disabled=${!!properties.disabled}
-                ?icon-right=${properties.iconRight}
                 @click=${action(`Click ${properties.variant}`)}
                 size=${properties.size || 'm'}
                 href=${ifDefined(properties.href)}

diff --git a/packages/button/test/button.test.ts b/packages/button/test/button.test.ts
@@ -64,24 +64,6 @@ describe('Button', () => {
         expect(el).to.not.be.undefined;
         expect(el.textContent).to.include('Button');
         expect(!((el as unknown) as { hasIcon: boolean }).hasIcon);
-        expect(!((el as unknown) as { iconRight: boolean }).iconRight);
-        await expect(el).to.be.accessible();
-    });
-    it('loads default w/ an icon-right', async () => {
-        const el = await fixture<Button>(
-            html`
-                <sp-button icon-right>
-                    Button
-                    <svg slot="icon"></svg>
-                </sp-button>
-            `
-        );
-
-        await elementUpdated(el);
-        expect(el).to.not.be.undefined;
-        expect(el.textContent).to.include('Button');
-        expect(((el as unknown) as { hasIcon: boolean }).hasIcon);
-        expect(((el as unknown) as { iconRight: boolean }).iconRight);
         await expect(el).to.be.accessible();
     });
     it('loads default only icon', async () => {