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

Replaced <br/> with <br> #5494

Merged
merged 1 commit into from
Jun 9, 2023

Conversation

iainbeeston
Copy link
Contributor

In regular HTML <br> is a void element, so it doesn't
need a closing tag or a closing slash. See the WhatWG
spec here: https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-br-element

Many of the shared templates used by devise use <br/>
to separate lines. To make that more correct I've replaced
them with <br> instead.

@p8
Copy link
Contributor

p8 commented May 31, 2022

Hi @iainbeeston, 👋
Looking at that WhatWG spec it also recommends using paragraphs instead of a <br> for separating label and input.
That's how it used to be before: b86c1c2
Not sure if we should change that back as well.

@iainbeeston
Copy link
Contributor Author

@p8 Thanks, I've replaced those with paragraph tags now. I've also updated the commit message to explicitly say that this reverts b86c1c2. Could you please take another look? (Especially at the HTML changes because I did them manually and it would be easy for me to make a mistake)

In regular HTML `<br>` is a void element, so it

Many of the shared templates used by devise use `<br/>`
to separate lines, which is invalid html because `<br>`
doesn't need a closing tag or a closing slash. See the
WhatWG spec here:
https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-br-element

Also, the WhatWG spec uses `<p>` tags to separate
`<label>` and `<input>` tags rather than `<br>`, see
here:
https://html.spec.whatwg.org/multipage/input.html

To clean this up I've replaced `<br/>` with paragraph
tags throughout all of the templates.

This reverts b86c1c2
@rafaelfranca rafaelfranca merged commit aca0b24 into heartcombo:main Jun 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants