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.

Sign up for GitHub

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

Jump to bottom

[fix]: prevent cursor jump for all text inputs #6941

Merged
merged 2 commits into from
Dec 13, 2021
Merged

[fix]: prevent cursor jump for all text inputs #6941

merged 2 commits into from
Dec 13, 2021

Conversation

skippednote
Copy link
Contributor

Fixes #6914

Before submitting the PR, please make sure you do the following

  • It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
  • Prefix your PR title with [feat], [fix], [chore], or [docs].
  • This message body should clearly illustrate what problems it solves.
  • Ideally, include a test that fails without this PR but passes with it.

Tests

  • Run the tests with npm test and lint the project with npm run lint

skippednote
skippednote commented Nov 19, 2021
View reviewed changes
src/compiler/compile/render_dom/wrappers/Element/Attribute.ts Outdated
@@ -204,7 +204,17 @@ export default class AttributeWrapper extends BaseAttributeWrapper {
if (this.is_input_value) {
const type = element.node.get_static_attribute_value('type');

if (type === null || type === '' || type === 'text' || type === 'email' || type === 'password') {
if (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm using MDN as a reference for input text types.
I've ignored DateTime as it's deprecated. Should we add it to support older browsers?

@Conduitry
Copy link
Member

Browsers will treat all unknown type values as type='text'. As I noted in #6914 (comment) I think it might be safer to use this code when the type is not one of the known non-text-like ones (checkbox, radio, file, etc.).

@skippednote
Copy link
Contributor Author

@Conduitry Thank you for the feedback. I'll reverse the condition.

@skippednote
Copy link
Contributor Author

@Conduitry I have reversed the condition to check for inputs that don't accept text values from users. I have also added all the types of input in the test to make sure all the cases are covered.

@Conduitry Conduitry merged commit c663104 into sveltejs:master Dec 13, 2021
@Conduitry Conduitry mentioned this pull request Dec 13, 2021
Open
Conduitry added a commit that referenced this pull request Dec 13, 2021
@Conduitry
update changelog
52549c4 
#6941 in fact addressesd a separate issue, specific to one-way binding
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Cursor jumps to end in search box in Safari
2 participants
@skippednote @Conduitry