Skip to content

Added tests for the classic input fields.#5202

Open
deather wants to merge 6 commits intoDogfalo:masterfrom
deather:test-input-fields
Open

Added tests for the classic input fields.#5202
deather wants to merge 6 commits intoDogfalo:masterfrom
deather:test-input-fields

Conversation

@deather
Copy link
Copy Markdown

@deather deather commented Sep 15, 2017

Proposed changes

I propose multiple tests about input fields #5023. I give you the list of field test :

  • text
  • password
  • email
  • url
  • tel
  • number
  • search
  • textarea

Tests added should cover all interactions that use could do.
The pull request may throw an error due to a bug about reset action on input. When the user click on reset button the label shouldn't be active.

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to change).

Checklist:

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@storm1er
Copy link
Copy Markdown

storm1er commented Sep 15, 2017

image

EDIT : same error on my computer

@deather
Copy link
Copy Markdown
Author

deather commented Sep 15, 2017

@storm1er I'm using node in version 7.5.0 and npm in version 5.4.1. Like I said in my previous comment, this test shouldn't work due to a bug.
The use case test is when a user fill an input and click on reset button. The input is clear but the label keep the class CSS active, but the input have no value so the label shouldn't keep the class CSS active.

@storm1er
Copy link
Copy Markdown

I'm using your code, aaannnnd it works well xD this is a missing feature ...
When i read "reset" I though "when empty by user" // keyup as materialize do it, not with a reset button

@deather
Copy link
Copy Markdown
Author

deather commented Sep 15, 2017

@storm1er : No problem. My explanations were not really clear :s.

@storm1er
Copy link
Copy Markdown

storm1er commented Sep 15, 2017

@deather please check this also =) deather#1

Just added the case where user fill inputs then empty them via user input, not via a reset button

@deather
Copy link
Copy Markdown
Author

deather commented Sep 15, 2017

@storm1er : it is good for me, I merge your work :)

@storm1er
Copy link
Copy Markdown

storm1er commented Sep 15, 2017

I realize, Is there somewhere on the web, an input[type=reset] ? (feels kinda deprecated for me)
It doesn't even render correctly with materializecss ...

@deather
Copy link
Copy Markdown
Author

deather commented Sep 15, 2017

It's not deprecated https://www.w3.org/TR/html5/forms.html#reset-button-state-(type=reset)

This input is just here to trigger the reset event, I think it's not really important to have a good render

@storm1er
Copy link
Copy Markdown

The real question was : Is this test-case necessary ?
I don't see any PR/Issue about this + MaterializeCss didn't stylized this button ...

If it is, we should also correct input[type="reset"]'s style.

@deather
Copy link
Copy Markdown
Author

deather commented Sep 15, 2017

I write this test because in the forms.js there are these lines :

// HTML DOM FORM RESET handling

@storm1er
Copy link
Copy Markdown

Well, I realized that input[type="button"] and input[type="submit"] wasn't stylized anyway : we're using .btn class instead.

-> working on fixing this reset bug =)

@storm1er storm1er mentioned this pull request Sep 18, 2017
8 tasks
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.

2 participants