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

feat(search-select-input): add isCondensed prop to SearchSelectInput … #2801

Merged
merged 2 commits into from
May 2, 2024

Conversation

Sarah4VT
Copy link
Contributor

@Sarah4VT Sarah4VT commented Apr 29, 2024

…component

Summary

Adds support for isCondensed prop on SearchSelectInput component to display a condensed layout.

Description

Designs
image

Results
image

I did notice that with a long input, the layout is a little off, but it was like this prior to my changes as well.
Existing code:
image

With my changes:
image

@Sarah4VT Sarah4VT self-assigned this Apr 29, 2024
@Sarah4VT Sarah4VT requested a review from a team April 29, 2024 21:05
Copy link

changeset-bot bot commented Apr 29, 2024

🦋 Changeset detected

Latest commit: c186e8c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 96 packages
Name Type
@commercetools-uikit/search-select-input Minor
@commercetools-uikit/select-utils Minor
@commercetools-uikit/search-select-field Minor
@commercetools-uikit/inputs Minor
@commercetools-uikit/async-creatable-select-input Minor
@commercetools-uikit/async-select-input Minor
@commercetools-uikit/checkbox-input Minor
@commercetools-uikit/creatable-select-input Minor
@commercetools-uikit/date-input Minor
@commercetools-uikit/date-range-input Minor
@commercetools-uikit/date-time-input Minor
@commercetools-uikit/localized-money-input Minor
@commercetools-uikit/money-input Minor
@commercetools-uikit/select-input Minor
@commercetools-uikit/selectable-search-input Minor
@commercetools-uikit/fields Minor
@commercetools-frontend/ui-kit Minor
@commercetools-uikit/async-creatable-select-field Minor
@commercetools-uikit/data-table-manager Minor
@commercetools-uikit/async-select-field Minor
@commercetools-uikit/creatable-select-field Minor
@commercetools-uikit/date-field Minor
@commercetools-uikit/date-range-field Minor
@commercetools-uikit/date-time-field Minor
@commercetools-uikit/money-field Minor
@commercetools-uikit/pagination Minor
@commercetools-uikit/select-field Minor
@commercetools-uikit/design-system Minor
@commercetools-uikit/calendar-time-utils Minor
@commercetools-uikit/calendar-utils Minor
@commercetools-uikit/hooks Minor
@commercetools-uikit/i18n Minor
@commercetools-uikit/localized-utils Minor
@commercetools-uikit/utils Minor
@commercetools-uikit/accessible-hidden Minor
@commercetools-uikit/avatar Minor
@commercetools-uikit/card Minor
@commercetools-uikit/collapsible-motion Minor
@commercetools-uikit/collapsible-panel Minor
@commercetools-uikit/collapsible Minor
@commercetools-uikit/constraints Minor
@commercetools-uikit/data-table Minor
@commercetools-uikit/field-errors Minor
@commercetools-uikit/field-label Minor
@commercetools-uikit/field-warnings Minor
@commercetools-uikit/grid Minor
@commercetools-uikit/icons Minor
@commercetools-uikit/label Minor
@commercetools-uikit/link Minor
@commercetools-uikit/loading-spinner Minor
@commercetools-uikit/messages Minor
@commercetools-uikit/notifications Minor
@commercetools-uikit/primary-action-dropdown Minor
@commercetools-uikit/progress-bar Minor
@commercetools-uikit/stamp Minor
@commercetools-uikit/tag Minor
@commercetools-uikit/text Minor
@commercetools-uikit/tooltip Minor
@commercetools-uikit/view-switcher Minor
@commercetools-uikit/accessible-button Minor
@commercetools-uikit/flat-button Minor
@commercetools-uikit/icon-button Minor
@commercetools-uikit/link-button Minor
@commercetools-uikit/primary-button Minor
@commercetools-uikit/secondary-button Minor
@commercetools-uikit/secondary-icon-button Minor
@commercetools-uikit/dropdown-menu Minor
@commercetools-uikit/localized-multiline-text-field Minor
@commercetools-uikit/localized-text-field Minor
@commercetools-uikit/multiline-text-field Minor
@commercetools-uikit/number-field Minor
@commercetools-uikit/password-field Minor
@commercetools-uikit/radio-field Minor
@commercetools-uikit/text-field Minor
@commercetools-uikit/time-field Minor
@commercetools-uikit/input-utils Minor
@commercetools-uikit/localized-multiline-text-input Minor
@commercetools-uikit/localized-rich-text-input Minor
@commercetools-uikit/localized-text-input Minor
@commercetools-uikit/multiline-text-input Minor
@commercetools-uikit/number-input Minor
@commercetools-uikit/password-input Minor
@commercetools-uikit/radio-input Minor
@commercetools-uikit/rich-text-input Minor
@commercetools-uikit/rich-text-utils Minor
@commercetools-uikit/search-text-input Minor
@commercetools-uikit/text-input Minor
@commercetools-uikit/time-input Minor
@commercetools-uikit/toggle-input Minor
@commercetools-uikit/spacings-inline Minor
@commercetools-uikit/spacings-inset-squish Minor
@commercetools-uikit/spacings-inset Minor
@commercetools-uikit/spacings-stack Minor
@commercetools-uikit/buttons Minor
@commercetools-uikit/spacings Minor
visual-testing-app Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Copy link

vercel bot commented Apr 29, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ui-kit ✅ Ready (Inspect) Visit Preview 💬 Add feedback May 2, 2024 1:28pm

@Sarah4VT Sarah4VT requested review from a team, jmcreasman, kterry1 and valoriecarli and removed request for a team April 29, 2024 21:05
@Sarah4VT
Copy link
Contributor Author

@commercetools/shield-team-design-system Not sure if you want me to change anything about the layout for that long input value example. It was working that way before my changes, so I wanted to be sure it wasn't intentional since it was not in my list of changes to implement.

@ddouglasz
Copy link
Contributor

@commercetools/shield-team-design-system Not sure if you want me to change anything about the layout for that long input value example. It was working that way before my changes, so I wanted to be sure it wasn't intentional since it was not in my list of changes to implement.

If this is not a blocker, Maybe we can create a ticket to see if we can have a follow-up fix from our end. Unless if it is something you think you can add to this PR :)
cc: @FilPob

Copy link
Contributor

@ddouglasz ddouglasz left a comment

Choose a reason for hiding this comment

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

Looks good 💯

@FilPob
Copy link

FilPob commented Apr 30, 2024

the condensed prop works correctly from my side. I can't reproduce the bug with the weird layout with long value in Storybook. @ddouglasz we can fix this in a follow up if you think that's better

@Sarah4VT
Copy link
Contributor Author

the condensed prop works correctly from my side. I can't reproduce the bug with the weird layout with long value in Storybook. @ddouglasz we can fix this in a follow up if you think that's better

Here is how I was able to create the bug. This is on the current version of UI kit. It's fine until you add the iconLeft.
Zight Recording 2024-04-30 at 04 35 40 PM

@Sarah4VT
Copy link
Contributor Author

@commercetools/shield-team-design-system I took a stab at trying to figure out the layout issue for a long value. I tried changing the width on the span inside WrapperWithIcon to be
width: ${calc(100% - ${props.selectProps.isCondensed ? 24 : 32}px)}
(I know the variables would need to be fixed up, but trying to do math with variables that are strings and include px I wasn't sure if it would work) There was still an element causing the span and the svg above it to be pushed up. I am happy to keep looking, but I am not sure what this input is doing so went ahead and opened a separate bug ticket and included a video of how to recreate it on the current code.
Components Inputs SelectInputs - SearchSelectInput ⋅ Storybook 2024-04-30 at 4 43 05 PM jpg

@ddouglasz
Copy link
Contributor

Thank you @Sarah4VT for giving it a try 🙏🏾. We will create a ticket and fix this as a follow up.

@Sarah4VT
Copy link
Contributor Author

Sarah4VT commented May 2, 2024

Thank you @Sarah4VT for giving it a try 🙏🏾. We will create a ticket and fix this as a follow up.

Yeah, I did look at it more yesterday. That input that is causing the problem is the input that is used when the user types into the search box. The problem is the parent element is a flex layout and because the span that contains the value is so long, that input is wrapping to the next line, then I think getting flex'd to take up the full space possibly. If you shrink the span down to the maximum amount that also allows space for the iconLeft, padding between iconLeft and the span, the clear button, and the search button (if they are all present), it will all go on one line and look correct. It's a bit of math to do in the code because you have a lot of conditions that may or may not be present. I probably could have gotten it working, but just wasn't sure if your team would have a different preferred solution.

Also I did create the github issue for this bug so feel free use that however you want.

@CarlosCortizasCT CarlosCortizasCT merged commit 91a86d8 into main May 2, 2024
7 checks passed
@CarlosCortizasCT CarlosCortizasCT deleted the PANGOLIN-3803-search-select-input branch May 2, 2024 13:45
@ct-changesets ct-changesets bot mentioned this pull request May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants