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

HTML and Live CRUD generators output invalid HTML (buttons nested in links) #5770

Open
angelikatyborska opened this issue Apr 2, 2024 · 0 comments · Fixed by #5820
Open

Comments

@angelikatyborska
Copy link
Contributor

Environment

  • Elixir version (elixir -v): 1.16.0
  • Phoenix version (mix deps): 1.7.11
  • Operating system: macOS 14.1

Actual behavior

When running mix phx.gen.live or mix phx.gen.html, the generated "index" and "show" pages contain a <button> element nested inside of an <a> element. This is not valid HTML and causes some problems when navigating with a keyboard. See the gif below - the "button link" can be focused twice: once as a link, and once as a button.

Those are the affected files:

Double focus problem gif

2024-04-02 19 20 25

Note: the page maybe doesn't look like you expect it to to because I didn't include Tailwind, and instead added Pico CSS. It shouldn't matter for this issue.

HTML validation error

Screenshot 2024-04-02 at 19 26 44

Expected behavior

I would expect the HTML from the generators to be valid. It should produce <a> elements with only text content, and the necessary Tailwind classes should be applied directly on the <a> element.

The problem of "semantic buttons" vs "visual buttons" is a common problem in web apps. Maybe it would make sense to introduce a new core component called link_button, that's semantically a link but visually a button? Or modify the current button component to change its semantics based on passed attributes. If a "href" attribute is given, make it semantically a link. If not, make it semantically a button?

phoebe100 added a commit to phoebe100/phoenix that referenced this issue May 17, 2024
SteffenDE added a commit that referenced this issue May 21, 2024
Nesting a button inside an anchor tag is not valid HTML. This change
introduces a new `link` attribute in the `button` core component that,
when present, will render the button as an anchor tag instead.

Fixes #5770.
References #5814.
SteffenDE added a commit that referenced this issue May 21, 2024
Nesting a button inside an anchor tag is not valid HTML. This change
introduces a new `link` attribute in the `button` core component that,
when present, will render the button as an anchor tag instead.

Fixes #5770.
References #5814.
SteffenDE added a commit that referenced this issue May 21, 2024
Nesting a button inside an anchor tag is not valid HTML. This change
introduces a new `link` attribute in the `button` core component that,
when present, will render the button as an anchor tag instead.

Fixes #5770.
References #5814.
SteffenDE added a commit that referenced this issue May 21, 2024
Nesting a button inside an anchor tag is not valid HTML. This change
introduces a new `link` attribute in the `button` core component that,
when present, will render the button as an anchor tag instead.

Fixes #5770.
References #5814.
SteffenDE added a commit that referenced this issue Nov 26, 2024
It is not allowed to render a button inside an <a> tag:

https://html.spec.whatwg.org/multipage/text-level-semantics.html#the-a-element

Therefore, we invert the order of the elements and nest a link inside a
button and forward any button clicks to the link.

Fixes #5770.
References #5814.
@SteffenDE SteffenDE reopened this Dec 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants