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

fix(ui): spacing between units and input updated #630

Merged
merged 8 commits into from
Nov 27, 2023
Merged

fix(ui): spacing between units and input updated #630

merged 8 commits into from
Nov 27, 2023

Conversation

RubirajAccenture
Copy link
Contributor

No description provided.

@RubirajAccenture RubirajAccenture force-pushed the fix/number-input-spacing branch 2 times, most recently from ade788f to d99247b Compare November 24, 2023 08:38
@RubirajAccenture RubirajAccenture changed the title draft spacing between units and input updated Nov 24, 2023
DenizSaganak
DenizSaganak reviewed Nov 24, 2023
View reviewed changes
Copy link
Contributor

@DenizSaganak DenizSaganak left a comment

Choose a reason for hiding this comment

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

I couldn't open the component in the storyBook.

packages/ui-library/src/components/forms/number-input/index.css.ts Outdated Show resolved Hide resolved
davidken91
davidken91 reviewed Nov 24, 2023
View reviewed changes
Copy link
Contributor

@davidken91 davidken91 left a comment

Choose a reason for hiding this comment

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

On line 34 of index.ts NumberInput we need to remove the question mark for the size to resolve the eslint issue

@davidken91
Copy link
Contributor

I'm not sure if this is within the scope of the ticket, but I don't the readonly styles are getting applied. Could we double check this?

davidken91
davidken91 reviewed Nov 24, 2023
View reviewed changes
Copy link
Contributor

@davidken91 davidken91 left a comment

Choose a reason for hiding this comment

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

On line 159 of index.css.ts (in number-input), we're getting an eslint error because it needs to be ${UserInput.Error.Focused};

@RubirajAccenture RubirajAccenture changed the title spacing between units and input updated fix(ui): spacing between units and input updated Nov 24, 2023
@RubirajAccenture RubirajAccenture merged commit 7f340ae into develop Nov 27, 2023
6 checks passed
@RubirajAccenture RubirajAccenture deleted the fix/number-input-spacing branch November 27, 2023 09:07
larserbach added a commit that referenced this pull request Nov 27, 2023
@RubirajAccenture @larserbach @davidken91
fix(ui): spacing between units and input updated (#630)
c96fd0b 
* draft

* feat(tokens): adds Forms.NumberInput.Input.TextContainer.ItemSpacing.[scale]

* feat(tokens): adds prefixSuffix colors

* fix(ui-library): fixing number input test by including missing size prop

* fix(ui-library): removing 'sm' parameter from text input to fix failing unit test

* fix(ui-library): making number input size property optional

* fix(ui-library): removing layout prop from radio input group story

---------

Co-authored-by: larserbach <[email protected]>
Co-authored-by: David Kennedy <[email protected]>
ChristianHoffmannS2 added a commit that referenced this pull request Dec 13, 2023
@ChristianHoffmannS2 @larserbach
@RubirajAccenture @davidken91 @DenizSaganak
535 radio rename tokens (#697)
b2a367e 
* fix(tokens): radio control tokens renamed

* fix(tokens): radio control tokens renaming. Mostly colors

* feat(tokens): adds radio borderwidth

* fix(tokens): border token

* radio control border tokens

* fix(tokens): adds error for active state

* fix(tokens): radio bordercolor

* radio control border tokens color

* fix(tokens): renames radio contentCol tokens

* fix(tokens): renames contentRow tokens

* fix(ui): spacing between units and input updated (#630)

* draft

* feat(tokens): adds Forms.NumberInput.Input.TextContainer.ItemSpacing.[scale]

* feat(tokens): adds prefixSuffix colors

* fix(ui-library): fixing number input test by including missing size prop

* fix(ui-library): removing 'sm' parameter from text input to fix failing unit test

* fix(ui-library): making number input size property optional

* fix(ui-library): removing layout prop from radio input group story

---------

Co-authored-by: larserbach <[email protected]>
Co-authored-by: David Kennedy <[email protected]>

* feat(ui-library): create buttonDisplay prop and type (#628)

* feat(ui-library): create buttonDisplay prop and type

* feat(ui-library): correct display none to default

* faet(ui-library): show block, inline-block and inline

* add block as display block

* feat(ui-library): add div for icon

* feat(ui-library): add vertical-align to center icons

* feart(ui-library): add a wrapper div

* feat(ui-library): add flex container

* feat(ui-library): add tests

* feat(ui-library): add gap

* feat(ui-library): fix loading state with new flex container

* fix(ui-library): add icon classes to iconMap

* fix(ui-library): unit tests are testing if css display attribute is really applied

* fix(ui-library): cleanup

* fix(ui-library): button display optional and better default

---------

Co-authored-by: christian.b.hoffmann <[email protected]>

* Switch to yarn pnp (#651)

* prepared yarn 4 pipeline

* yarn 4 with corepack

* renamed jobs and removed push-trigger

* removed unused yml files

* updated read.me

* fix(tokens): radioGroup tokens

* fix: radio inactive hover and pressed 
adjusted icon sizes

* fix(tokens): apply new tokens 0001

* fix: radio - css nesting

* fix: updates comments on radio

* fix: radio comment

* adds: prefixsuffix colors

* ignore yarn stuff

* removed unused files

* deleted checkbox file

---------

Co-authored-by: larserbach <[email protected]>
Co-authored-by: RubirajAccenture <[email protected]>
Co-authored-by: David Kennedy <[email protected]>
Co-authored-by: DenizSaganak <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@DenizSaganak DenizSaganak DenizSaganak left review comments

@davidken91 davidken91 davidken91 approved these changes

@kaikdi kaikdi Awaiting requested review from kaikdi

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@RubirajAccenture @davidken91 @DenizSaganak @larserbach