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

Preserve placeholder text on focus #209

Closed
wants to merge 11 commits into from

Conversation

mlms13
Copy link

@mlms13 mlms13 commented May 23, 2014

This series of commits gives the plugin the option to behave more like the placeholder implementation in Chrome and Firefox. Placeholder text remains visible until characters are entered into the input. See #92 for more info.

mlms13 added 5 commits May 23, 2014 10:18
This will make it easier to override them in a way that applies to both
regular inputs and passwords.
To trigger the feature, call the placeholder plugin with an object that
sets a preserveOnFocus property to true.  Real documentation coming
after I fix a couple issues with cursor placement.
This also makes some attempt to prevent selecting placeholder text by
repositioning the caret on clicks. Additionally, options now extend a
settings object that is available throughout the whole plugin.
By attempting to reset the placeholder text on keyup, the placeholder
text will re-appear if you use backspace to clear a form field.
@amerikan
Copy link
Collaborator

@mlms13 seems like a good thing to have. The code has fallen behind. Also, we don't need an option, it should be the default setting. All modern browsers (except ie 10/11) behave this way (they preserve the placeholder on focus). Can you please update? Thanks.

@amerikan
Copy link
Collaborator

@mlms13 ping

@mlms13
Copy link
Author

mlms13 commented Dec 31, 2014

Hey, I'm about to jump on a plane. I'll see if I have a chance to make your suggested changes then. I'm excited the library is being maintaned again. :)

Conflicts:
	README.md
	jquery.placeholder.js
Per the comments in the pull request, the new behavior should be the default. All modern browsers choose to leave the placeholder text until you start typing, so jquery.placeholder will emulate that behavior.
@mlms13
Copy link
Author

mlms13 commented Jan 10, 2015

It should be up-to-date now, but I haven't tested it since merging master into this branch. I'll run it in some browsers on Monday to make sure it still behaves as expected.

@ghost
Copy link

ghost commented Jan 22, 2015

@mlms13 Were you able to solve this?

@mlms13
Copy link
Author

mlms13 commented Jan 22, 2015

Just fixed a typo that was causing the grunt build to fail. I tested in IE8 and IE9, and as far as I can tell, everything looks ok to me.

@amerikan
Copy link
Collaborator

@mlms13 I haven't had time to test it yet, but I do see you use .on method to attach an event handler, however according to the jquery docs, .on() was introduced in jQuery 1.7, the jquery-placeholder plugin itself at this point must be compatible with jQuery 1.6 so I don't think it'll work. I'm already thinking for jquery-placeholder 2.0 to drop 1.6 support, but for now it needs to support it.

@gabskoro
Copy link

Tested on IE11 and nothing, it still disappeares

@mlms13
Copy link
Author

mlms13 commented Apr 10, 2015

@gabskoro I don't think the jquery-placeholder plugin polyfills IE11, because it natively supports the placeholder attribute. And, unfortunately, its native implementation makes the text disappear on focus. 😞

@mlms13
Copy link
Author

mlms13 commented May 6, 2015

Looks like my branch is conflicting again. I'm happy to keep it up to date if there's a chance that this will be merged.

@amerikan
Copy link
Collaborator

amerikan commented May 6, 2015

@mlms13 yes please update.

@gabskoro see issue #204, ideally we would want to allow a developer to override native if desired, though still have to figure out how to do that

@Alex1990
Copy link
Contributor

Alex1990 commented Jun 6, 2015

If a user pastes some text by the Paste command in context menu, the keydown or keyup events cannot be triggered.

This feature needs an event like input event in IE10+ and other modern browers. See this article A near-perfect oninput shim for IE 8 and 9. Of course, you can write a simplified version.

@mlms13
Copy link
Author

mlms13 commented Jun 10, 2015

@Alex1990 ... ignore my original comment... I got it now. Yes, since we're no longer targeting focus, we need more intelligent ways to figure out if the input contains text.

@amerikan If there's anything else I need to do, feel free to add it to this checklist.

  • Fix merge conflicts
  • Be smarter about checking for input and clearing the placeholder
  • Test in some virtual machines

I'm not including the request to override IE 10's behavior, because that seems like a bigger issue that changes the behavior of this plugin (ie feature detection will no longer be enough)... a bit outside of the scope of this PR.

@mlms13
Copy link
Author

mlms13 commented Jun 18, 2015

If anyone is still interested in this feature, my fork is up to date with the latest changes. It hasn't been tested recently, and it looks like a weird merge happened with the setPlaceholder method. Since I no longer need to support old IE, and there doesn't seem to be interest in merging this upstream, I probably won't continue to keep it up to date, but if anyone else wants to maintain it, it's all yours. :)

@amerikan
Copy link
Collaborator

@mlms13 I believe this PR got automatically closed because the master branch was deleted in favor of the gh-pages branch which serves as the project demo. So if you would like to make this changes to the gh-pages branch I can test then merge. I really think it's a good feature and mimics the behavior of the modern browsers excluding IE of course.

@mlms13
Copy link
Author

mlms13 commented Jun 18, 2015

Ah, makes sense. I'll try to fix up the weirdness I'm seeing in the setPlaceholder method, and I'll do some testing and reopen against gh-pages.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants