Improve frontend checkout forms html #2416
Merged
Conversation
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
They are useless since inputs have with: 100% and will still go on a new line
The only reason why they currently are <p> elements is to let them have the <p> padding, which instead should be set via css.
padding: none is not valid css
There's no reason to keep that class, we should set padding with css
Remove that extra span element in favor of using css to add this sign near the required fields label. This needed to add a class (field-required) to required fields contanier div. JS address is also changed so that instead of toggling span.required visibility it toggle field-required class on field container.
Input and select inside fields need to be display block so they will go on a new line without the need of the br element. Also, removes a useless html entity.
On mobile it was looking very bad, with label centered and input on the left.
kennyadsl
added
changelog:solidus_frontend
tvdeyen
approved these changes
Nov 30, 2017
There was a problem hiding this comment.
Usually I would say that we should not hard code number values into scss files and use variables instead. Also px should not be used, but rem.
BUT: this is the default frontend that no one will ever use for their store anyways. So, I don’t care and consider these changes good changes 👍
Thanks
mamhoff
approved these changes
Dec 1, 2017
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
These changes are useful under the assumption that we should try to reduce the html provided.
This change reduces the amount of code that needs to be changed after cloning frontend views, which are often taken as guideline/initial template when developing custom views.
Also, it adds a line of css to fix how checkout content is looking on mobile:
From:
To: