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

495 clear button #530

Merged
merged 3 commits into from
Jul 30, 2014
Merged

495 clear button #530

merged 3 commits into from
Jul 30, 2014

Conversation

anselmbradford
Copy link
Member

Fixes #495

  • Adds search input manager for handling input clear button—little (x).
  • Disables autocomplete because field highlight color can’t be
    overridden.
  • Removes unused data-list attributes.
  • Adds tests for clearing the input fields.

Also:

  • Normalizes button and form id/class names using the specificity
    principle (general --> specific).
  • Removes unused classes from submit/reset buttons.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8aaae67 on 495-clear-button into ffbcaa4 on master.

@anselmbradford
Copy link
Member Author

clearbtn-updates

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling e5b4b77 on 495-clear-button into ffbcaa4 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling f2309f2 on 495-clear-button into ffbcaa4 on master.

@coveralls
Copy link

Coverage Status

Coverage remained the same when pulling 8452289 on 495-clear-button into e7750c6 on master.

- Adds search input manager for handling input clear button—little (x).
- Disables autocomplete because field highlight color can’t be
overridden.
- Removes unused data-list attributes.
- Adds tests for clearing the input fields.
- Normalized button and form id/class names using the specificity
principle.
- Removes unused classes from submit/reset buttons.
@anselmbradford
Copy link
Member Author

Note: This isn't available in IE8, but IE8 support could be added in another PR. An issue for that should be opened when this is merged. It gracefully degrades in IE8 and the clear all button can be used to clear all fields, so it's not a priority to add support.

@anselmbradford
Copy link
Member Author

@monfresh Ready for review and merge! Thx

I think autocomplete is too useful a feature to turn off just for
cosmetic reasons.
@monfresh
Copy link
Member

@anselmbradford Reviewed and pushed changes. It looks great, except for turning off autocomplete, which I disagree with. I think autocomplete is too useful a feature to turn off just for cosmetic reasons. I turned it back on in my latest changes and made a fix for asset compilation.

@coveralls
Copy link

Coverage Status

Coverage increased (+46.21%) when pulling 0531f89 on 495-clear-button into 9887280 on master.

@anselmbradford
Copy link
Member Author

@monfresh Okay, fair enough. It's not purely aesthetics though, the autocomplete in Chrome makes the yellow background stick when programmatically clearing the input so you can't see the placeholder text anymore:

screen shot 2014-07-30 at 8 39 57 am

Here's the chromium discussions about it: https://code.google.com/p/chromium/issues/detail?id=46543

I'm trying to find a workaround.

@monfresh
Copy link
Member

Weird. I don't see that on my computer. What version of Chrome are you using?

@monfresh
Copy link
Member

Ah, I see it now. I wasn't selecting from the autocomplete. Is that just on Chrome? I don't see it in Safari or Firefox.

@anselmbradford
Copy link
Member Author

Yeah, pretty sure it's just Chrome.

@monfresh
Copy link
Member

It's crazy that the bug has been there for 4 years and no one has taken ownership of it.

@monfresh
Copy link
Member

I think we can still go ahead with this PR while you find a workaround for Chrome. Let me know if it's good to go and I'll fix conflicts and merge.

@anselmbradford
Copy link
Member Author

Sounds good. I'll open an issue after it's merged.

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.

Should individual search fields have an "x" to clear them?
3 participants