-
Notifications
You must be signed in to change notification settings - Fork 46
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
Add section about forms and submit buttons #320
Closed
Closed
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ This document outlines the way we write markup and why. See our [house style doc | |
- [Semantics](#semantics) | ||
- [Follow the HTML 4 outline model for heading levels](#follow-the-html-4-outline-model-for-heading-levels) | ||
- [Templates](#templates) | ||
- [Forms](#forms) | ||
|
||
## HTML vs XHTML | ||
|
||
|
@@ -121,3 +122,23 @@ We *don't* do this: | |
## Templates | ||
|
||
Templates should, where possible, be written using [DustJS](http://www.dustjs.com/) or [Handlebars](http://handlebarsjs.com/). Use of linting is encouraged to prevent common errors. For projects using DustJS [Dustmite](https://www.npmjs.com/package/dustmite) should be used. Dustmite will prevent any syntax errors. It also enforces consistent style in your template code and can catch more subtle errors like usage of the potentially unsafe `{@if}` dust helper. | ||
|
||
## Forms | ||
|
||
On modern browsers, `button type="submit"` provides a superset of functionality when compared to `input type=submit`. | ||
|
||
We do this: | ||
|
||
```html | ||
<form> | ||
<button type="submit">Search</button> | ||
</form> | ||
``` | ||
|
||
We *don't* do this: | ||
|
||
```html | ||
<form> | ||
<input type="submit">Search</input> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We definitely don't do that, since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indeed, well spotted 🏅 |
||
</form> | ||
``` |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting! Could you join a resource that list the superset of functionality. I only know that you can then nest other elements inside it. In "Form Design Pattern" it is stated that this variant have the side effect in older version of IE to submit the value of all the buttons to the server, regardless of which one was clicked, so you'll need to know which button was clicked so you can determine the right course of action to take. Maybe this is worth mentioning.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @morgaan - I just offered to write this so we could rid of an issue that has been open for two years. To be honest, I'm not familiar with the implications, I was just summarising a conversation that we had in Slack years ago.
If you're more familiar with this than me, feel free to amend the text that I've submitted!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @josebolos - First of all apologies that I did not mentioned it was an existing issue pending for long time. I thought it was coming out from work you being busy with lately.
I do not know much more TBH, I just remembered these lines from the book and I have been using mostly the input variant over the years, unless I had the need to nest stuffs in there. This is what is maybe meant by superset of functionality.
IMO I would let it open to developers to pick from, depending on their use case.
Reading the "We don't do this" looks like the input variant is bad practice which I doubt it is, at least according to Adam Silver, author of the book.
This may though be worth mentioning that in case of using the
button
variant that it does not fall into the edge cases category.What do you think? We could also bring this topic into the next FE open space to see what is the fellowship opinion on that matter 2 years later.
In any case thank you for offering to write this ☀️. I plan to come up with a draft of the few tips I got from the book at writing forms, and already applied to NatCar project. This one could end up being a guideline chapter into the Playbook about writing forms and or a part in the Elements design system styleguide. It could also include the outcome of the current discussion.
Cheers!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@morgaan sounds great, thanks very much!
I also vaguely remember @davidpauljunior having some thoughts on button vs input, do you have some David? :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we know which older version of IE? If it's old enough that it can't support TLS 1.2 then it won't be clicking any buttons on any of our sites because it won't be able to access them at all: https://github.com/springernature/frontend-playbook/blob/master/practices/graded-browser-support.md#browsers-without-tls-12-support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still in IE11 as far as I understand. But it also seems that you should be safe if you provide the
button
withname
andvalue
attributes. In such case we should emphase this in the Playbook if using thebutton
variant.