feat(Button, OrderedList, UnorderedList): add expressive styles #8477

jnm2377 · 2021-04-21T21:28:47Z

Went back and forth about whether or not to create a different component for this or just adding a prop to the existing code. Ultimately, since we felt the style changes were minor (with type style being the main thing changing), we went with just adding a prop to toggle a className. Totally open to feedback if the team thinks there is a better way to do this.

New

adds expressive prop for Button, OrderedList and UnorderedList
adds IconOnly ghost button to playground story so that isSelected knob will actually do something

Testing / Reviewing

View the Playground story knobs for Button to toggle expressive styles
View Nested story knobs for OrderedList and UnorderedList to toggle expressive styles

netlify · 2021-04-21T21:45:35Z

Deploy preview for carbon-elements ready!

Built with commit 61d08d8

https://deploy-preview-8477--carbon-elements.netlify.app

netlify · 2021-04-21T22:04:10Z

Deploy preview for carbon-components-react ready!

Built without sensitive environment variables with commit 61d08d8

https://deploy-preview-8477--carbon-components-react.netlify.app

shixiedesign · 2021-04-22T03:09:51Z

So. Excited. Thank you Josefina! A few possible changes but I wanna wait a few more hours so I can discuss with other designers. More soon!

shixiedesign

Hey! So this is looking very good. The only 1 minor request for change is: is it possible to disallow button size option for field and small when expressive is toggled on? The smaller buttons don't look good with 16px label and we don't wanna make them available:

So if based on the new size prop values, the expressive button size options would be:

lg = 48
xl = 64
2xl = 80

aagonzales · 2021-04-23T16:15:02Z

The only 1 minor request for change is: is it possible to disallow button size option for field and small when expressive is toggled on? The smaller buttons don't look good with 16px label and we don't wanna make them available

This actually bring up an interesting point. Should we just not provide code for those smaller expressive options. We don't want those to exist at all correct? @shixiedesign

tw15egan · 2021-04-23T17:41:57Z

@aagonzales We could enforce it on the code side if someone passed down xs, sm, field or small, and they have passed down isExpressive, it will just map to lg

packages/react/src/components/Button/Button-story.js

…pressive-list-btn

jnm2377 · 2021-05-04T19:32:22Z

@aagonzales @tw15egan @emyarod Updated the PR. Expressive buttons now default to a min of 48px height, but can be used with larger sizes. Icon only expressive buttons also can be used in larger sizes, but if they're not expressive the default sizing stays the same (i.e. doesn't go above 48px).

See recording below for testing:

Screen.Recording.2021-05-04.at.2.28.01.PM.mov

aagonzales · 2021-05-04T20:32:12Z

Oops guys I may have been wrong about the icon-only button. Talking to @jeanservaas now.

jeanservaas · 2021-05-04T20:33:15Z

@aagonzales @emyarod @tw15egan @jnm2377 @andreancardona

Ug. OK, sorry, trying to get up to speed with the conversation. Are we talking about having:

icon only button that is 48px with a 16px icon
and
icon only button that is 48px with a 20px icon (expressive)

I got your question mixed up Anna... I thought you were asking about "fluid" icon buttons. But yes, we DO need to give them a. 48px icon only button with a 20px icon

So for the expressive button variants, we would make these BUTTON variants with the 16px type

lg = 48px with 16px type
xl = 64px with 16px type
2xl 80px with 16px type

And only one ICON ONLY BUTTON variant expressive variant

lg = 48px with 16px type

It would not make sense to make a fluid icon only button, because the type on our fluid (64 and 80px buttons) is not vertically centered. Icon only buttons do not bleed to the edge of containers (unless @shixiedesign has a use case i'm not aware of)

emyarod

given that the expressive theme will only affect certain button variants, would it make more sense to have a separate Expressive story rather than having some knobs in the Playground story that have no effect on certain button variants?

jnm2377 · 2021-05-11T19:56:01Z

@aagonzales @emyarod @tw15egan @jeanservaas this is ready for re-review. We added an expressive specific story, and updated icon-only expressive button to be only 48px. 👍🏽

aagonzales

Ok I think this all looks correct now! Thanks for hanging in there y'all.

tw15egan

A few comments but looking good 👍🏻

packages/components/src/components/button/_button.scss

packages/react/src/components/Button/Button-story.js

packages/react/src/components/Button/Button.js

jnm2377 · 2021-05-11T21:24:56Z

@tw15egan updated 👍🏽

tw15egan

Looks great!

emyarod

looks good to me

jnm2377 added 6 commits April 21, 2021 15:12

feat: add expressive prop to btn

946231a

feat: add expressive styles

c8e1192

docs: add expressive knob to playground

c46c1a6

feat: add expressive prop to lists

d0702c7

feat: add expressive list styles

ff2958f

docs: add expressive knob to list stories

c2af92a

jnm2377 requested a review from a team as a code owner April 21, 2021 21:28

jnm2377 requested review from emyarod, dakahn and andreancardona April 21, 2021 21:28

emyarod requested a review from shixiedesign April 21, 2021 21:29

emyarod added the status: visual review 🔍 label Apr 21, 2021

andreancardona mentioned this pull request Apr 21, 2021

Sprint 8: Quarantine Quetzal 🦜 #8398

Closed

shixiedesign reviewed Apr 22, 2021

View reviewed changes

chore: update tests

1ec23a3

jnm2377 requested a review from a team as a code owner April 22, 2021 20:18

shixiedesign approved these changes Apr 22, 2021

View reviewed changes

jnm2377 requested review from a team and aagonzales and removed request for a team April 23, 2021 13:15

tw15egan reviewed Apr 23, 2021

View reviewed changes

packages/react/src/components/Button/Button-story.js Outdated Show resolved Hide resolved

jnm2377 added 2 commits April 29, 2021 16:36

Merge branch 'main' of github.com:carbon-design-system/carbon into ex…

8c45d5d

…pressive-list-btn

fix: update prop name and sizing

a84705e

jnm2377 requested a review from tw15egan April 29, 2021 22:15

chore: update tests

c41ed91

jnm2377 requested review from tw15egan and emyarod May 4, 2021 19:33

emyarod reviewed May 4, 2021

View reviewed changes

fix: expressive story and icon only btn

6e982ee

jnm2377 requested a review from emyarod May 11, 2021 19:54

aagonzales approved these changes May 11, 2021

View reviewed changes

tw15egan reviewed May 11, 2021

View reviewed changes

jnm2377 added 3 commits May 11, 2021 16:16

chore: remove comment

353d35a

chore: remove comment

94aea9f

fix: v10 sizing for expressive story

50320bd

jnm2377 removed the status: visual review 🔍 label May 11, 2021

jnm2377 requested a review from tw15egan May 11, 2021 21:25

tw15egan approved these changes May 11, 2021

View reviewed changes

Merge branch 'main' into expressive-list-btn

37b6f92

jnm2377 added the status: one more review 👀 label May 11, 2021

dakahn approved these changes May 12, 2021

View reviewed changes

emyarod approved these changes May 12, 2021

View reviewed changes

Merge branch 'main' into expressive-list-btn

eea7c9e

tw15egan added status: ready to merge 🎉 and removed status: one more review 👀 status: ready for review 👀 labels May 12, 2021

Merge branch 'main' into expressive-list-btn

61d08d8

kodiakhq bot merged commit c829406 into carbon-design-system:main May 12, 2021

emyarod mentioned this pull request May 13, 2021

fix(button): explicitly set width for expressive theme #8686

Merged

tw15egan mentioned this pull request May 13, 2021

v10.35.0 #8636

Closed

22 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(Button, OrderedList, UnorderedList): add expressive styles #8477

feat(Button, OrderedList, UnorderedList): add expressive styles #8477

jnm2377 commented Apr 21, 2021

netlify bot commented Apr 21, 2021 •

edited

Loading

netlify bot commented Apr 21, 2021 •

edited

Loading

shixiedesign commented Apr 22, 2021

shixiedesign left a comment •

edited

Loading

aagonzales commented Apr 23, 2021

tw15egan commented Apr 23, 2021

jnm2377 commented May 4, 2021 •

edited

Loading

aagonzales commented May 4, 2021

jeanservaas commented May 4, 2021 •

edited

Loading

emyarod left a comment

jnm2377 commented May 11, 2021

aagonzales left a comment

tw15egan left a comment

jnm2377 commented May 11, 2021

tw15egan left a comment

emyarod left a comment

feat(Button, OrderedList, UnorderedList): add expressive styles #8477

feat(Button, OrderedList, UnorderedList): add expressive styles #8477

Conversation

jnm2377 commented Apr 21, 2021

Testing / Reviewing

netlify bot commented Apr 21, 2021 • edited Loading

netlify bot commented Apr 21, 2021 • edited Loading

shixiedesign commented Apr 22, 2021

shixiedesign left a comment • edited Loading

Choose a reason for hiding this comment

So if based on the new size prop values, the expressive button size options would be:

aagonzales commented Apr 23, 2021

tw15egan commented Apr 23, 2021

jnm2377 commented May 4, 2021 • edited Loading

aagonzales commented May 4, 2021

jeanservaas commented May 4, 2021 • edited Loading

emyarod left a comment

Choose a reason for hiding this comment

jnm2377 commented May 11, 2021

aagonzales left a comment

Choose a reason for hiding this comment

tw15egan left a comment

Choose a reason for hiding this comment

jnm2377 commented May 11, 2021

tw15egan left a comment

Choose a reason for hiding this comment

emyarod left a comment

Choose a reason for hiding this comment

netlify bot commented Apr 21, 2021 •

edited

Loading

netlify bot commented Apr 21, 2021 •

edited

Loading

shixiedesign left a comment •

edited

Loading

jnm2377 commented May 4, 2021 •

edited

Loading

jeanservaas commented May 4, 2021 •

edited

Loading