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

fix: match form component borders, update foundation #314

Merged
merged 16 commits into from
Jan 9, 2025

Conversation

wbarbee
Copy link
Contributor

@wbarbee wbarbee commented Jan 6, 2025

Summary

Radio button and dropdown borders did not match the rest of the form components, making them stand out.
Fix bug in dropdown where disabled

  • on hover was transparent.

    ADO Story or GitHub Issue Link

    N/A

    Figma Link

    N/A

    Notes

    • radio and dropdown previously used --kd-color-border-ui-default for border-color, which was not matching the rest of the form components. this PR changes to use --kd-color-border-forms-default

    To Do

    • ensure that these changes are reflected in figma designs and approved by UX
    Screenshot 2025-01-07 at 10 25 42 AM Screenshot 2025-01-07 at 10 24 11 AM

    Checklist

    • Used Conventional Commit messages as outlined in the contributing guide.
      • Noted breaking changes (if any).
    • Documented/updated all props, events, slots, parts, etc with JSDoc.
      • Ran the analyze command to update Storybook docs.
    • Added/updated Stories with controls to cover all variants.
    • Ran test locally to address any failures.
    • Added/updated the Figma link for the Story's Design tab.
    • Added any new component exports to the src/index.ts file

    Testing Instructions

    Provide guidance to reviewers/testers here.

    Screenshots

    Previous:
    Screenshot 2025-01-06 at 11 10 33 AM

    Screenshot 2025-01-06 at 10 21 34 AM Screenshot 2025-01-06 at 11 10 16 AM Screenshot 2025-01-07 at 10 10 52 AM

    Updated:

    Screenshot 2025-01-06 at 11 34 00 AM Screenshot 2025-01-06 at 11 33 54 AM


    Screenshot 2025-01-06 at 11 32 59 AM
    Screenshot 2025-01-06 at 11 32 49 AM

  • Copy link

    netlify bot commented Jan 6, 2025

    Deploy Preview for shidoka-applications ready!

    Name Link
    🔨 Latest commit 7b4756d
    🔍 Latest deploy log https://app.netlify.com/sites/shidoka-applications/deploys/677fda45eb92e3000805e94d
    😎 Deploy Preview https://deploy-preview-314--shidoka-applications.netlify.app
    📱 Preview on mobile
    Toggle QR Code...

    QR Code

    Use your smartphone camera to open QR code link.

    To edit notification comments on pull requests, go to your Netlify site configuration.

    @wbarbee wbarbee marked this pull request as ready for review January 7, 2025 16:27
    @wbarbee wbarbee requested review from a team January 7, 2025 16:27
    @brian-patrick-3
    Copy link
    Contributor

    Any particular reason the datepicker inputs have a different background color vs the other form components?

    @wbarbee
    Copy link
    Contributor Author

    wbarbee commented Jan 7, 2025

    Any particular reason the datepicker inputs have a different background color vs the other form components?

    nope. i think those were the original tokens used in the designs. fixed those

    Screenshot 2025-01-07 at 10 34 37 AM

    @brian-patrick-3
    Copy link
    Contributor

    I noticed Dropdown also has a different background color, in light mode

    @wbarbee
    Copy link
    Contributor Author

    wbarbee commented Jan 7, 2025

    I noticed Dropdown also has a different background color, in light mode

    fixed this as well. we're using a <span> wrapper when the input isn't searchable, so the placeholder is not styled the same as other placeholders? when i add the class and the placeholder styling when applicable, i'm getting an accessibility error.

    is this what you were avoiding @brian-patrick-3?

    <span class=${this.value === '' ? 'placeholder' : ''}>
    ${this.multiple
    ? this.placeholder
    : this.value === ''
    ? this.placeholder
    : this.text}
    </span>

    Screenshot 2025-01-07 at 10 47 56 AM Screenshot 2025-01-07 at 10 47 19 AM

    @brian-patrick-3
    Copy link
    Contributor

    I'm guessing the input placeholder attribute does not get flagged for accessibility, but this does being actual text.

    @srpriyankashetty
    Copy link

    As discussed with @wbarbee , the dropdown is showing as transparent in dark mode, dev is looking into it.
    image

    @wbarbee
    Copy link
    Contributor Author

    wbarbee commented Jan 8, 2025

    As discussed with @wbarbee , the dropdown is showing as transparent in dark mode, dev is looking into it.

    this color token has an opacity value (the 0.3) that makes it semi transparent but is set for the background of the dropdown

      in figma. need to confirm with UX

      Screenshot 2025-01-08 at 9 01 04 AM

    @wbarbee
    Copy link
    Contributor Author

    wbarbee commented Jan 8, 2025

    spoke with ux, the double background color was not unintentional. setting to Background/UI/Hollow/Default

    Copy link

    @srpriyankashetty srpriyankashetty left a comment

    Choose a reason for hiding this comment

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

    Verified Radio button color, dropdown options background color and looks fine
    Verified the field background color for drop down, multi select dropdown, Date fields and Text fields

    @wbarbee wbarbee merged commit 2b696a0 into next Jan 9, 2025
    8 of 9 checks passed
    @wbarbee wbarbee deleted the fix/form-component-borders branch January 9, 2025 14:19
    @brian-patrick-3
    Copy link
    Contributor

    🎉 This PR is included in version 2.0.0-next.41 🎉

    The release is available on:

    Your semantic-release bot 📦🚀

    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.

    3 participants