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

Bug: calcite-input no longer has the same height as calcite-action #2736

Closed
2 tasks
AdelheidF opened this issue Aug 4, 2021 · 11 comments
Closed
2 tasks

Bug: calcite-input no longer has the same height as calcite-action #2736

AdelheidF opened this issue Aug 4, 2021 · 11 comments
Assignees
Labels
1 - assigned Issues that are assigned to a sprint and a team member. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. design Issues that need design consultation prior to development.

Comments

@AdelheidF
Copy link

With next.261. It was fine with beta.60.

calcite-input and calcite-action have both scale=s.

image

Actual Behavior

Expected Behavior

Reproduction Steps or Sample

Relevant Info

Version: @esri/calcite-components@<version>

  • CDN
  • NPM package
@AdelheidF AdelheidF added bug Bug reports for broken functionality. Issues should include a reproduction of the bug. 0 - new New issues that need assignment. needs triage Planning workflow - pending design/dev review. labels Aug 4, 2021
@bstifle
Copy link

bstifle commented Aug 5, 2021

@asangma currently the actions are at 32px, 48px, 64px.

Would you be down with getting actions down to 24, 32, 44 like buttons, inputs, selects, etc?

@julio8a
Copy link

julio8a commented Aug 5, 2021

I see this one is still in the refactor to-do. We can prioritize it to match the height scale of all other components.

Would there be any reason this component should NOT match the scale as all other components?

@jcfranco jcfranco added help wanted Issues that the core team needs help with in a sprint. and removed needs triage Planning workflow - pending design/dev review. labels Aug 5, 2021
@jcfranco jcfranco added this to the Sprint 8/2 – 8/13 milestone Aug 5, 2021
@jcfranco jcfranco added design Issues that need design consultation prior to development. and removed help wanted Issues that the core team needs help with in a sprint. labels Aug 5, 2021
@julio8a
Copy link

julio8a commented Aug 5, 2021

If there are no objections with updating the heights of calcite-action: S (24px) , M (32px), and L (44px) we can have this one updated this sprint by prioritizing calcite-action Figma/tailwind refactor. Will wait until 8/9 for feedback/comments on this.

@asangma
Copy link
Contributor

asangma commented Aug 5, 2021

@bstifle will need to see how that affects..kinda everything.

It would have broad effects in existing implementations.

As a note, when Action is not showing text and is just the icon, medium would be 32x32 which I believe is smaller than suggested mobile touch size.

It's also pretty small hit areas for widely separated interfaces like left and right action-bars.

@bstifle
Copy link

bstifle commented Aug 6, 2021

@asangma agreed. pretty much all the layout components utilize action.

another way around this would be to not use actions in tandem with inputs, per the OG issue. use a native button and style accordingly

@AdelheidF
Copy link
Author

AdelheidF commented Aug 6, 2021

another way around this would be to not use actions in tandem with inputs, per the OG issue. use a native button and style accordingly

This is a breaking change for a bunch of our components. Also, I'd have to figure out how to make the focus states look the same.

@AdelheidF
Copy link
Author

AdelheidF commented Aug 6, 2021

What about adding a size XS for calcite-action that would then match calcite-input? Still a breaking change, but at least it's easy to fix.

@macandcheese
Copy link
Contributor

Couldn't actions that are slotted in those locations just use scale="l", just a 4px change from current?

@caripizza caripizza added 1 - assigned Issues that are assigned to a sprint and a team member. and removed 0 - new New issues that need assignment. labels Aug 11, 2021
@asangma
Copy link
Contributor

asangma commented Aug 13, 2021

I see two things going on here.

  1. We could add styles in Input to target a slotted Action in the same way we're doing it for Button. (we do this is other places effectively)
  2. Input currently sets heights based on scale. We should consider letting the height be defined by the font sizes and padding.
  3. We can also update Input styling to be more responsive to its slotted content. (we do this is other places effectively)

@asangma
Copy link
Contributor

asangma commented Aug 13, 2021

Note that 1. is probably the simplest approach though I think 2. should be considered at some point as a separate discussion. And 3. is probably a broader philosophical suggestion toward more flexible componentry.

@AdelheidF
Copy link
Author

AdelheidF commented Aug 16, 2021

Solving this issue now by applying a height of 24px to the action to make it match the input.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1 - assigned Issues that are assigned to a sprint and a team member. bug Bug reports for broken functionality. Issues should include a reproduction of the bug. design Issues that need design consultation prior to development.
Projects
None yet
Development

No branches or pull requests

7 participants