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.

Sign up for GitHub

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

Jump to bottom

Allow for lang attribute in <mj-body> #2355

Closed
rootwork opened this issue Sep 11, 2021 · 5 comments
Closed

Allow for lang attribute in <mj-body> #2355

rootwork opened this issue Sep 11, 2021 · 5 comments

Comments

@rootwork
Copy link

Is your feature request related to a problem? Please describe.

From @hteumeuleu https://github.com/hteumeuleu/email-guidelines#lang:

Defining the lang of the HTML content helps assistive technologies like screen readers to pick the right voice to read the content. The lang attribute needs to be defined with a valid language tag on the <html> opening tag. But because some email clients (especially webmails) remove the <html> element, the lang attribute also needs to be set on a wrapping element within the <body>.

<!DOCTYPE html>
<html lang="en">
  <head></head>
  <body>
    <div lang="en">
    </div>
  </body>
</html>

MJML currently accepts a lang attribute on <mjml>, which it places on the root <html> element, but it does not accept one on the <mj-body> element, which would map to the containing <div> element that it creates.

Describe the solution you'd like

Allow for lang on <mj-body>.

Describe alternatives you've considered

The only current workaround that I've found is to nest a <div> within every <mj-column> and set the language there, which is a pain.

Additional context

This is arguably an a11y bug, but since it's adding a new enhancement I used the feature issue template.

I'd be happy to do a PR for this if the maintainers agree with the idea.
@GarryFlemings
Copy link
Contributor

Alternative

The lang attribute on <mjml> also adds the lang annotation on the div after <mj-body>. MJML has no lang attribute on <mj-body>.

Comments

Good suggestion! And, taking nothing away from it ...

This option is appropriate if no user would want, for example, lang=en in one place and lang=fr in the other. Maybe that's expectable.

This option abstracts a little complexity from MJML users, an added value for MJML.

This option makes MJML more maintainable because the team is less likely to get questions about the difference between the two attributes.

This option keeps MJML more consistent with HTML. MDN documents the lang attribute for <html> (our equivalent being mjml), but not for <body> (ours is mj-body).

@iRyusa
Copy link
Member

iRyusa commented Sep 13, 2021

👍 I think we should push globalDatas (and probably rename this with a smarter name) into bodyHelpers here https://github.com/mjmlio/mjml/blob/master/packages/mjml-core/src/index.js#L268

Tell me if you still want to work on this @rootwork I might have some times to open a PR for that at the end of the week

@rootwork rootwork mentioned this issue Nov 9, 2021
Closed
kmcb777 pushed a commit that referenced this issue Jan 13, 2022
ref #2396 #2355 move body tag from skeleton to mj-body, add lang on w…
1245156 
…rapping div
@iRyusa
Copy link
Member

iRyusa commented Mar 30, 2022

I think this should be good in 4.12 👍

@iRyusa iRyusa closed this as completed Mar 30, 2022
@Showcase-Joz
Copy link

hey @iRyusa , can i check that a global lang tag has been added? our (company) system uses "mjml-browser": "^4.10.4", not the current 4.14.x

and i cant find any documentation to support that lang has been added. thus i don't know if it's worth asking our sys admin to up our mjml version 🤔

@iRyusa
Copy link
Member

iRyusa commented Sep 18, 2023

You can check release note it's available since 4.13 https://github.com/mjmlio/mjml/releases/tag/v4.13.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@rootwork @iRyusa @Showcase-Joz @GarryFlemings