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

Text input tweaks ✨ #652

Merged
merged 17 commits into from Dec 19, 2019
Merged

Text input tweaks ✨ #652

merged 17 commits into from Dec 19, 2019

Conversation

emplums
Copy link

@emplums emplums commented Dec 18, 2019

This PR cleans up a few things around the TextInput and FilteredSearch components.

  • Updates FilteredSearch docs to make sure users are aware that the component must be used with a TextInput and Dropdown in order for it to work correctly
  • Removes unnecessary native attributes from prop types for TextInput
  • Removes unused input-icon & input-no-icon classes
  • Fixes double border in safari (ty @T-Hugs!)
  • Applies background color on wrapper instead of inner input
  • Removes type="search" from docs - we use type="text" on GitHub.com
  • Corrects margin/padding to match Primer CSS
  • Moves most of the styles to the Wrapper element, uses omit and pick from @styled-system/props to pass the appropriate props to the Wrapper and Input. This fixes a bunch of issues with having COMMON on the input (margin getting applied in the wrong place, etc).

@emplums emplums requested a review from T-Hugs December 18, 2019 16:59
@vercel
Copy link

vercel bot commented Dec 18, 2019

This pull request is being automatically deployed with ZEIT Now (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://zeit.co/primer/primer-components/nqmr9cqz6
🌍 Preview: https://primer-components-git-text-input-tweaks.primer.now.sh

@vercel vercel bot temporarily deployed to staging December 18, 2019 17:00 Inactive
@vercel vercel bot temporarily deployed to staging December 18, 2019 17:15 Inactive
@vercel vercel bot temporarily deployed to staging December 18, 2019 17:20 Inactive
@vercel vercel bot temporarily deployed to staging December 18, 2019 17:22 Inactive
@emplums emplums removed the request for review from T-Hugs December 18, 2019 17:23
@emplums emplums changed the title Text input tweaks ✨ [WIP] Text input tweaks ✨ Dec 18, 2019
@vercel vercel bot temporarily deployed to staging December 18, 2019 18:56 Inactive
@emplums emplums requested a review from T-Hugs December 18, 2019 18:57
@emplums emplums changed the title [WIP] Text input tweaks ✨ Text input tweaks ✨ Dec 18, 2019
@vercel vercel bot temporarily deployed to staging December 18, 2019 18:59 Inactive
@vercel vercel bot temporarily deployed to staging December 18, 2019 19:05 Inactive
@vercel vercel bot temporarily deployed to staging December 18, 2019 19:07 Inactive
@vercel vercel bot temporarily deployed to staging December 18, 2019 19:23 Inactive
Copy link
Contributor

@BinaryMuse BinaryMuse 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 to me!

const inputClasses = classnames(icon ? 'input-icon' : 'input-no-icon')
const hasIcon = !!icon
const wrapperProps = pick(rest)
const inputProps = omit(rest)
Copy link
Contributor

Choose a reason for hiding this comment

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

Neat 👍

Copy link
Author

Choose a reason for hiding this comment

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

it's extremely useful for wrapper components like this!!

@emplums emplums merged commit 13018ce into release-15.2.0 Dec 19, 2019
@emplums emplums deleted the text-input-tweaks branch December 19, 2019 19:51
@T-Hugs T-Hugs mentioned this pull request Dec 19, 2019
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.

None yet

2 participants