Design System: Refactor IconButton and update documentation #66774

L-M-K-B · 2023-04-18T13:54:26Z

Which issue(s) does this PR fix?:
Fixes #66600

Special notes for your reviewer:
Note: All sizes of plain button elements correspond with icon sizes.

size (Figma - code)	status	icon	padding	hover size
small - xs	before	12px
	after	12px	4px	20px

grafana-ui Storybook => IconButton Examples
in Code: packages/grafana-ui/src/components/IconButton/IconButton.tsx

Before:	After:

size (Figma - code)	status	icon	padding	hover size
N/A - sm	before	14px		~24px
	after	14px	4px	22px

Explore => Elasticsearch query editor
in Code: public/app/plugins/datasource/elasticsearch/components/QueryEditor/QueryEditorRow.tsx

Before:	After:

size (Figma - code)	status	icon	padding	hover size
medium - md	before	16px		~25px
	after	16px	4px	24px

Explore => any query Editor
in Code: public/app/core/components/QueryOperationRow/QueryOperationAction.tsx

Before:	After:

size (Figma - code)	status	icon	padding	hover size
N/A - lg	before	18px		~23px
	after	18px	4px	26px

grafana-ui Storybook => IcoButton Examples
in Code: packages/grafana-ui/src/components/IconButton/IconButton.tsx

Before:	After:

size (Figma - code)	status	icon	padding	hover size
large - xl	before	24px		~28px
	after	24px	4px	32px

Home => new navigation
in Code: public/app/core/components/AppChrome/MegaMenu/NavBarMenu.tsx

Before:	After:

size (Figma - code)	status	icon	padding	hover size
N/A - xxl	before	36px		~48px
	after	24px	4px	32px

grafana-ui Storybook => PageToolbar Examples
in Code: packages/grafana-ui/src/components/PageLayout/PageToolbar.tsx

Before:	After:

size (Figma - code)	status	icon	padding	hover size
N/A - xxxl	before	48px
	after	24px	4px	32px

Before:	After:

JoaoSilvaGrafana

It seems like xl icons have grown a lot in hover state... Is our recommendation that those change to lg icons now? I feel like we have to address the hover size changes because they are quite drastic...

L-M-K-B · 2023-05-05T14:20:42Z

We definitely need to discuss the sizes for xl and lg that's why I added it as a topic to our Saga weekly on Monday.

L-M-K-B · 2023-05-11T09:24:18Z

public/app/features/alerting/unified/components/DynamicTable.tsx

@@ -125,9 +125,8 @@ export const DynamicTable = <T extends object>({
                <div className={cx(styles.cell, styles.expandCell)}>
                  <IconButton
                    aria-label={`${isItemExpanded ? 'Collapse' : 'Expand'} row`}
-                    size="lg"


Before: After:

Changed to md due to consistency reasons (the chevron used above is also size md).

L-M-K-B · 2023-05-11T09:28:16Z

We definitely need to discuss the sizes for xl and lg that's why I added it as a topic to our Saga weekly on Monday.

Since the padding according to Figma especially for larger sizes would break the UI @staton-hyse11, @JoaoSilvaGrafana and I decided to set the padding to 4px for each size and let the IconSize define how large the hover state is becoming.

JoaoSilvaGrafana

I think it looks better now with the 4px!

JoaoSilvaGrafana · 2023-05-11T16:01:28Z

packages/grafana-ui/src/components/IconButton/IconButton.tsx

+const getStyles = stylesFactory((theme: GrafanaTheme2, size, variant: IconButtonVariant) => {
+  // overall size of the IconButton on hover
+  // the value 8 originates from 2*4px and letting the IconSize generally decide on the hoverSize
+  const hoverSize = getSvgSize(size) + 8;


we should make it use the theme here, so 2 * theme.spacing(0.5)

Unfortunately theme.spacing() is returning a string including 'px'.
I could do something like parseInt(theme.spacing(0.5).substring(0)) but this is cumbersome. What do you think?

Oh thats true... We could also use theme.spacing.gridSize which returns 8? And keep this in mind when we do spacing tokens

I changed it to theme.spacing.gridSize, modified the related comment in the code and added a reminder to the related epic for spacing tokens.

packages/grafana-ui/src/components/IconButton/IconButton.story.tsx

JoaoSilvaGrafana

Lgtm! 🙌

* refactor: remove unnecessary styling and adjust to button styling * refactor: improve story * refactor: use new default styling for border radius * refactor: add missing pseudo classes and clean up * refactor: repair disabled styling and add to story * refactor: clean up and apply styles defined in figma * refactor: set hover background in a pseudo-element * refactor: unify large sizes * refactor: add information for further use * refactor: add comment * refactor: comments and clean up import * refactor: add changes after code review * refactor: replace some bad example code in documentation * refactor: update comment Co-authored-by: Joao Silva <[email protected]> * refactor: add changes requested in review * refactor: move deprecation warning * refactor: replace padding * refactor: remove local styling * refactor: create separate stories for different examples * refactor: change style of story * refactor: replace absolute value by variable * Update toggles_gen.go --------- Co-authored-by: Joao Silva <[email protected]>

L-M-K-B added 3 commits April 18, 2023 15:45

refactor: remove unnecessary styling and adjust to button styling

c151b6b

refactor: improve story

4edaf78

refactor: use new default styling for border radius

c584610

L-M-K-B added area/frontend add to changelog no-backport Skip backport of PR design-system labels Apr 18, 2023

L-M-K-B added this to the 9.5.x milestone Apr 18, 2023

L-M-K-B added 8 commits April 18, 2023 16:33

refactor: add missing pseudo classes and clean up

4acfb78

refactor: repair disabled styling and add to story

19aba85

refactor: clean up and apply styles defined in figma

2378f13

refactor: set hover background in a pseudo-element

890f9b3

refactor: unify large sizes

f5bb0cc

Merge branch 'main' into laura/unify-IconButton-styling-with-Button

d640c00

Merge branch 'main' into laura/unify-IconButton-styling-with-Button

6bcae65

refactor: add information for further use

17e23e2

eledobleefe mentioned this pull request May 3, 2023

ID 260316: [Programmatic Access - Simon - Value Mapping]: Aria-label is not defined for "Remove" button under "Value mapping" dialog. #66346

Closed

refactor: add comment

e618957

L-M-K-B marked this pull request as ready for review May 3, 2023 14:01

L-M-K-B requested review from a team as code owners May 3, 2023 14:01

L-M-K-B requested review from leventebalogh, joshhunt, eledobleefe, a team and JoaoSilvaGrafana and removed request for a team May 3, 2023 14:01

L-M-K-B changed the title ~~Laura/unify icon button styling with button~~ Design System: Refactor of IconButton and update documentation May 3, 2023

L-M-K-B changed the title ~~Design System: Refactor of IconButton and update documentation~~ Design System: Refactor IconButton and update documentation May 3, 2023

refactor: move deprecation warning

d6f124c

JoaoSilvaGrafana reviewed May 5, 2023

View reviewed changes

L-M-K-B added 3 commits May 10, 2023 14:02

Merge branch 'main' into laura/unify-IconButton-styling-with-Button

1923605

refactor: replace padding

f4275d3

refactor: remove local styling

cf28f1f

L-M-K-B requested a review from a team May 11, 2023 09:19

L-M-K-B commented May 11, 2023

View reviewed changes

L-M-K-B requested a review from JoaoSilvaGrafana May 11, 2023 09:24

Merge main branch into feat branch

2f7ce5a

L-M-K-B requested a review from grafanabot as a code owner May 11, 2023 14:20

grafanabot added the area/backend label May 11, 2023

JoaoSilvaGrafana reviewed May 11, 2023

View reviewed changes

L-M-K-B added 2 commits May 12, 2023 10:02

Merge branch 'main' into laura/unify-IconButton-styling-with-Button

8e8f175

refactor: create separate stories for different examples

20a5c35

JoaoSilvaGrafana reviewed May 12, 2023

View reviewed changes

packages/grafana-ui/src/components/IconButton/IconButton.story.tsx Outdated Show resolved Hide resolved

refactor: change style of story

9ea630a

L-M-K-B requested a review from JoaoSilvaGrafana May 12, 2023 13:26

L-M-K-B and others added 2 commits May 12, 2023 16:41

refactor: replace absolute value by variable

f77eabc

Update toggles_gen.go

024e57c

JoaoSilvaGrafana approved these changes May 15, 2023

View reviewed changes

L-M-K-B merged commit e7dc357 into main May 15, 2023

L-M-K-B deleted the laura/unify-IconButton-styling-with-Button branch May 15, 2023 11:57

zerok modified the milestones: 9.5.x, 10.1.x Jun 29, 2023

ricky-undeadcoders modified the milestones: 10.1.x, 10.1.0 Aug 1, 2023

chenrui333 mentioned this pull request Aug 24, 2023

grafana 10.1.0 Homebrew/homebrew-core#140321

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Design System: Refactor IconButton and update documentation #66774

Design System: Refactor IconButton and update documentation #66774

L-M-K-B commented Apr 18, 2023 •

edited

Loading

JoaoSilvaGrafana left a comment

L-M-K-B commented May 5, 2023

L-M-K-B May 11, 2023 •

edited

Loading

L-M-K-B commented May 11, 2023

JoaoSilvaGrafana left a comment

JoaoSilvaGrafana May 11, 2023

L-M-K-B May 12, 2023

JoaoSilvaGrafana May 12, 2023

L-M-K-B May 12, 2023 •

edited

Loading

JoaoSilvaGrafana left a comment

Design System: Refactor IconButton and update documentation #66774

Design System: Refactor IconButton and update documentation #66774

Conversation

L-M-K-B commented Apr 18, 2023 • edited Loading

JoaoSilvaGrafana left a comment

Choose a reason for hiding this comment

L-M-K-B commented May 5, 2023

L-M-K-B May 11, 2023 • edited Loading

Choose a reason for hiding this comment

L-M-K-B commented May 11, 2023

JoaoSilvaGrafana left a comment

Choose a reason for hiding this comment

JoaoSilvaGrafana May 11, 2023

Choose a reason for hiding this comment

L-M-K-B May 12, 2023

Choose a reason for hiding this comment

JoaoSilvaGrafana May 12, 2023

Choose a reason for hiding this comment

L-M-K-B May 12, 2023 • edited Loading

Choose a reason for hiding this comment

JoaoSilvaGrafana left a comment

Choose a reason for hiding this comment

L-M-K-B commented Apr 18, 2023 •

edited

Loading

L-M-K-B May 11, 2023 •

edited

Loading

L-M-K-B May 12, 2023 •

edited

Loading