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.

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

Indigo Themes: Updates to Toast, Stepper, Rating #1249

Merged
merged 22 commits into from
Jul 2, 2024

Conversation

simeonoff
Copy link
Collaborator

No description provided.

@simeonoff simeonoff marked this pull request as ready for review June 18, 2024 10:13
@AnjiManova

This comment was marked as resolved.

@AnjiManova

This comment was marked as resolved.

@andiesm813
Copy link

Rating

The label padding should be 2px, not 4px. The right padding of the symbols should be removed. There are sizes for the symbols provided in the kit, which are - small -18x18;
medium - 24x24 (default one) @andiesm813 confirm this pleases; and
large - 30x30 (mentioning them because in Angular they are not the same) The row gap should be 4px, not 5px.

RATING
looks good to me @AnjiManova, the symbols sizes that is.
The rest of the component looks good! ✅

@andiesm813
Copy link

TOAST
Looking good! ;) ✅

@andiesm813
Copy link

STEPPER
It looks fine to me... Except for the "content" part, that should have more padding.... but since that's in a slot, it's not something we control, correct?

image

@desig9stein
Copy link
Contributor

STEPPER It looks fine to me... Except for the "content" part, that should have more padding.... but since that's in a slot, it's not something we control, correct?

image

We can, I change it to 16px as it is in the UI kit

@AnjiManova
Copy link

AnjiManova commented Jun 27, 2024

Stepper

  • Min-height of the vertical lines should be 24px
  • Min-width of the horizontal lines should be 40px ✅ implemented

Light & Dark Mode

  • Invalid state - the subtitle text color should be grays.700 and should uses other var I guess ✅ implemented
image

…the min with of the horizontal separator to 40px
@desig9stein
Copy link
Contributor

desig9stein commented Jun 27, 2024

Stepper

  • Min-height of the vertical lines should be 24px
  • Min-width of the horizontal lines should be 40px

Light & Dark Mode

  • Invalid state - the subtitle text color should be grays.700 and should uses other var I guess
image

@AnjiManova For the min-height of the vertical separator we should create another issue since it is not a mater of just fixing the theme, it requires changes in the component animation.

@AnjiManova
Copy link

Stepper

  • Min-height of the vertical lines should be 24px
  • Min-width of the horizontal lines should be 40px

Light & Dark Mode

  • Invalid state - the subtitle text color should be grays.700 and should uses other var I guess
image

@AnjiManova For the min-height of the vertical separator we should create another issue since it is not a mater of just fixing the theme, it requires changes in the component animation.

Agreed, a new issue to be then.

SisIvanova
SisIvanova previously approved these changes Jul 2, 2024
@simeonoff simeonoff merged commit 2b08ef2 into master Jul 2, 2024
5 checks passed
@simeonoff simeonoff deleted the simeonoff/indigo-themes branch July 2, 2024 08:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants