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

Jump to bottom

release-4.0.0 (HDS-2191) fix button styles across #1251

Merged
merged 1 commit into from
Apr 25, 2024
Merged

release-4.0.0 (HDS-2191) fix button styles across #1251

merged 1 commit into from
Apr 25, 2024

Conversation

mrTuomoK
Copy link
Contributor

@mrTuomoK mrTuomoK commented Apr 4, 2024 •
edited
Loading

Description

Renews the old Button component and makes changes accordingly across the rest of the components, adds possibility to theme the Button and changes some props to enums. Also adds the possibility to use icons on components by giving them correct classes (what was wanted for buttons, not to contain the spans (example on core side) )

Demos
react
core
site

You can also check the other components too which use Button-component

Related Issues

HDS-2191
HDS-1117
HDS-2195
HDS-2208
HDS-2209
HDS-2184
HDS-1831
HDS-2024

How Has This Been Tested?

  • local machine

Add to changelog

  • Added needed line to changelog

@mrTuomoK mrTuomoK changed the title (HDS-2191) fix button styles across release-4.0.0 (HDS-2191) fix button styles across Apr 9, 2024
@mrTuomoK mrTuomoK added the Breaking changes Issue or pull request that includes breaking changes. label Apr 10, 2024
@mrTuomoK mrTuomoK force-pushed the HDS-2191-fix-button branch 4 times, most recently from e705bb7 to 67ebb22 Compare April 15, 2024 09:08
@mrTuomoK mrTuomoK marked this pull request as ready for review April 15, 2024 09:35
@mrTuomoK mrTuomoK requested a review from a team April 15, 2024 16:27
@mrTuomoK mrTuomoK force-pushed the HDS-2191-fix-button branch from 5548ac8 to de5caa5 Compare April 16, 2024 05:40
@Riippi
Copy link
Contributor

Riippi commented Apr 16, 2024

Padding seems weird in these react storybook button examples:

  • with icons
  • Loading
  • Small

Also buttons in datepicker and accordion have odd paddings on buttons when focused.

In Core storybook buttons look normal.

@mrTuomoK
Copy link
Contributor Author

mrTuomoK commented Apr 17, 2024 •
edited
Loading

Padding seems weird in these react storybook button examples:

  • with icons
  • Loading
  • Small

Also buttons in datepicker and accordion have odd paddings on buttons when focused.

In Core storybook buttons look normal.

Hmm. Everything is according to designs, could you @SillanmakiA check them (check also Accordion and DatePicker)? @Riippi the Button paddings and designs have changed a bit, but we'll check everything again. Oh the loki-test images take some stupid empty space in themselves from some container around the buttons at least (so those are not any paddings or other in the components themselves)

NikoHelle
NikoHelle suggested changes Apr 17, 2024
View reviewed changes
Copy link
Contributor

@NikoHelle NikoHelle left a comment

Choose a reason for hiding this comment

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

This makes buttons some much clearer! Excellent work! Left some comments about minor typos and excessive functions.

Squash the second commit! :)

packages/react/src/components/button/Button.stories.tsx Outdated Show resolved Hide resolved
packages/react/src/components/button/Button.stories.tsx Show resolved Hide resolved
packages/react/src/components/button/Button.stories.tsx Outdated Show resolved Hide resolved
.editorconfig Outdated Show resolved Hide resolved
packages/core/src/components/button/button.css Show resolved Hide resolved
packages/react/src/components/button/Button.stories.tsx Outdated Show resolved Hide resolved
packages/core/src/components/button/button.stories.js Show resolved Hide resolved
packages/core/src/components/button/buttonVariants.css Show resolved Hide resolved
CHANGELOG.md Show resolved Hide resolved
packages/react/src/components/button/Button.tsx Outdated Show resolved Hide resolved
@mrTuomoK mrTuomoK force-pushed the HDS-2191-fix-button branch 2 times, most recently from acf104c to 208d4b0 Compare April 17, 2024 11:30
@SillanmakiA
Copy link
Contributor

Padding seems weird in these react storybook button examples:

  • with icons
  • Loading
  • Small

Also buttons in datepicker and accordion have odd paddings on buttons when focused.

In Core storybook buttons look normal.

Can you elaborate a bit on what you found weird? The buttons seem to be the same between react and core and as designed.

@Riippi
Copy link
Contributor

Riippi commented Apr 18, 2024

Padding seems weird in these react storybook button examples:

  • with icons
  • Loading
  • Small

Also buttons in datepicker and accordion have odd paddings on buttons when focused.
In Core storybook buttons look normal.

Hmm. Everything is according to designs, could you @SillanmakiA check them (check also Accordion and DatePicker)? @Riippi the Button paddings and designs have changed a bit, but we'll check everything again. Oh the loki-test images take some stupid empty space in themselves from some container around the buttons at least (so those are not any paddings or other in the components themselves)

Maybe there is some issue on my machine if things look ok for others?

Screenshot 2024-04-18 at 14 37 20
Screenshot 2024-04-18 at 14 38 13
Screenshot 2024-04-18 at 14 37 48

@Riippi
Copy link
Contributor

Riippi commented Apr 18, 2024

Padding seems weird in these react storybook button examples:

  • with icons
  • Loading
  • Small

Also buttons in datepicker and accordion have odd paddings on buttons when focused.
In Core storybook buttons look normal.

Can you elaborate a bit on what you found weird? The buttons seem to be the same between react and core and as designed.

Replied on Tuomo's message about this.

Riippi
Riippi approved these changes Apr 19, 2024
View reviewed changes
Copy link
Contributor

@Riippi Riippi left a comment

Choose a reason for hiding this comment

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

👍

@mrTuomoK mrTuomoK force-pushed the HDS-2191-fix-button branch from 208d4b0 to 7fbcddf Compare April 22, 2024 10:59
@mrTuomoK mrTuomoK force-pushed the HDS-2191-fix-button branch 5 times, most recently from e16b0da to 203fbfd Compare April 23, 2024 12:00
@laurakarhu laurakarhu added this to the HDS 4.0.0 milestone Apr 23, 2024
@mrTuomoK mrTuomoK requested a review from NikoHelle April 24, 2024 06:32
NikoHelle
NikoHelle approved these changes Apr 25, 2024
View reviewed changes
Copy link
Contributor

@NikoHelle NikoHelle left a comment

Choose a reason for hiding this comment

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

I'll leave padding etc verifications to designers. Otherwise looks very good.

@mrTuomoK mrTuomoK force-pushed the HDS-2191-fix-button branch from 203fbfd to cd82519 Compare April 25, 2024 08:07
@mrTuomoK mrTuomoK merged commit 3168b8c into release-4.0.0 Apr 25, 2024
7 checks passed
@mrTuomoK mrTuomoK deleted the HDS-2191-fix-button branch April 25, 2024 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@NikoHelle NikoHelle NikoHelle approved these changes

@Riippi Riippi Riippi approved these changes

Assignees
No one assigned
Labels
Breaking changes Issue or pull request that includes breaking changes.
Projects
None yet
Milestone
HDS 4.0.0
Development

Successfully merging this pull request may close these issues.
5 participants
@mrTuomoK @Riippi @SillanmakiA @NikoHelle @laurakarhu