Skip to content

feat: ✨ sd-button with sd-badge sample#420

Merged
Vahid1919 merged 9 commits intomainfrom
feat/sd-button-sample
Oct 6, 2023
Merged

feat: ✨ sd-button with sd-badge sample#420
Vahid1919 merged 9 commits intomainfrom
feat/sd-button-sample

Conversation

@Vahid1919
Copy link
Copy Markdown
Contributor

Description:

Added a samples story to showcase the sd-button working with sd-badge. Closes #341

Definition of Reviewable:

PR notes: Irrelevant elements should be removed.

  • Documentation is created/updated
  • Stories (features, a11y) are created/updated
  • relevant tickets are linked
  • PR is assigned to project board

Copy link
Copy Markdown
Contributor

@MarcMatthiae MarcMatthiae left a comment

Choose a reason for hiding this comment

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

@coraliefeil Correct me if I´m wrong, but as I remember it, the badge is supposed to "grow to the left" when the texts gets longer.

@Vahid1919 That is the exact point I got stuck the last time, as you have to change the positioning of the badge in the button component. See FIGMA.

image

@coraliefeil
Copy link
Copy Markdown
Contributor

Yes growing to the left would be good ... if possible in code ... if that is causing too much stress, centered would be ok as well. only to the right growing would look weird I guess.

@mariohamann
Copy link
Copy Markdown
Contributor

@Vahid1919 What is the current state of this ticket?

@Vahid1919
Copy link
Copy Markdown
Contributor Author

Vahid1919 commented Sep 25, 2023

@Vahid1919 What is the current state of this ticket?

@mariohamann I have to look into the growing behavior to see what I can do. Currently it is growing to the right which is not the behaviour we want.

Moving this back to "In Progress" as this is something that is waiting on me.

@Vahid1919
Copy link
Copy Markdown
Contributor Author

Yes growing to the left would be good ... if possible in code ... if that is causing too much stress, centered would be ok as well. only to the right growing would look weird I guess.

@coraliefeil @MarcMatthiae Can confirm that the current behavior is centered, not right.

@coraliefeil
Copy link
Copy Markdown
Contributor

coraliefeil commented Sep 26, 2023

Well, if it is growing from it’s center then that’s ok ...

Nevertheless this comments still neds fixing:

Pls set badge to inverted: true when showing it together with an inverted button.

Pls add a story for a tertiary button - icon only - where the badge is positioned top-right to the icon. (https://www.figma.com/file/frKFVz9UBKAts...)

THX

@Vahid1919 Vahid1919 assigned coraliefeil and unassigned Vahid1919 Sep 27, 2023
@Vahid1919
Copy link
Copy Markdown
Contributor Author

@coraliefeil Requesting re-review after a fix for the tertiary buttons :)
I also reworked the sample to closely resemble the Figma document.

@Vahid1919 Vahid1919 self-assigned this Sep 27, 2023
@Vahid1919
Copy link
Copy Markdown
Contributor Author

Vahid1919 commented Oct 4, 2023

@van-nguyen-ht Assigning this to you for the Chromatic tests/review :) I addressed Colarie's comments in Chromatic.

@karlbaumhauer karlbaumhauer requested review from karlbaumhauer and removed request for karlbaumhauer October 4, 2023 14:06
@van-nguyen-ht
Copy link
Copy Markdown
Contributor

@Vahid1919 va
I've checked on the change and accepted :)

@abudd1094
Copy link
Copy Markdown
Contributor

abudd1094 commented Oct 5, 2023

@Vahid1919 @mariohamann
The problem is the "translate: 50% -50%" on the badge slot in the button. I tried using absolute positioning to do the desired translate, then the badge will grow to the left. Since the badge is not perfect (there are margins being applied for the smaller variants in the story to achieve the desired position), I see this as a valid solution. You could try relative units if that is a factor (10% or 15% thereabouts):

image

@Vahid1919
Copy link
Copy Markdown
Contributor Author

@van-nguyen-ht I aligned a small margin difference I noticed between the inverted and non-inverted tertiary buttons. Please review :)

@Vahid1919 Vahid1919 merged commit da1f63a into main Oct 6, 2023
@Vahid1919 Vahid1919 deleted the feat/sd-button-sample branch October 6, 2023 12:49
karlbaumhauer pushed a commit that referenced this pull request Oct 6, 2023
# [@solid-design-system/components-v1.17.0](components/1.16.0...components/1.17.0) (2023-10-06)

### Features

* ✨ sd-button with sd-badge sample ([#420](#420)) ([da1f63a](da1f63a))
yoezlem pushed a commit that referenced this pull request Oct 9, 2023
yoezlem pushed a commit that referenced this pull request Oct 9, 2023
# [@solid-design-system/components-v1.17.0](components/1.16.0...components/1.17.0) (2023-10-06)

### Features

* ✨ sd-button with sd-badge sample ([#420](#420)) ([da1f63a](da1f63a))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

feat: ✨ Add sample for sd-button with sd-badge

7 participants