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

[Markdown] Remove inline styles from JS reference #4118

Merged
merged 1 commit into from
Apr 15, 2021

Conversation

wbamberg
Copy link
Collaborator

@wbamberg wbamberg commented Apr 15, 2021

This PR is intended to remove all uses of the style attribute from the JS docs (with one exception).

According to my script, these are all the files in the JS docs that use the style attribute:

https://github.com/mdn/content/tree/main/files/en-us/web/javascript/enumerability_and_ownership_of_properties/index.html
https://github.com/mdn/content/tree/main/files/en-us/web/javascript/equality_comparisons_and_sameness/index.html
https://github.com/mdn/content/tree/main/files/en-us/web/javascript/eventloop/index.html
https://github.com/mdn/content/tree/main/files/en-us/web/javascript/guide/details_of_the_object_model/index.html
https://github.com/mdn/content/tree/main/files/en-us/web/javascript/reference/global_objects/function/apply/index.html
https://github.com/mdn/content/tree/main/files/en-us/web/javascript/reference/global_objects/math/cos/index.html
https://github.com/mdn/content/tree/main/files/en-us/web/javascript/reference/global_objects/math/index.html
https://github.com/mdn/content/tree/main/files/en-us/web/javascript/reference/global_objects/math/log1p/index.html
https://github.com/mdn/content/tree/main/files/en-us/web/javascript/reference/global_objects/regexp/exec/index.html
https://github.com/mdn/content/tree/main/files/en-us/web/javascript/typed_arrays/index.html

In most cases just removing the style seemed to have no effect or didn't make the pages any worse.

In one case (the long note inside https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply#using_apply_to_chain_constructors) I just removed the whole note, as it said "The Object.create() method used above is relatively new...." which I don't think is true any more.

The most involved case was https://developer.mozilla.org/en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties, which uses inline styles to stop the layout from breaking when you have huge tables. I've reorganized and broken up the tables to avoid this (and I think the result is much clearer although I still find this page a bit baffling). I hope I didn't introduce any errors here, it's a bit intricate.

The case I didn't address was https://developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness. One excuse is that the styles are all inside tables, and I expect that we will keep at least some tables in HTML even when we are in Markdown, so we don't need to fix them now. But perhaps I should try to attack it anyway, inline styles are not a good idea anyway.

@wbamberg wbamberg requested a review from a team as a code owner April 15, 2021 00:41
@wbamberg wbamberg requested review from sideshowbarker and removed request for a team April 15, 2021 00:41
@github-actions
Copy link
Contributor

Preview URLs

Flaws

URL: /en-US/docs/Web/JavaScript/EventLoop
Title: Concurrency model and the event loop
on GitHub

No flaws! 🎉


URL: /en-US/docs/Web/JavaScript/Guide/Details_of_the_Object_Model
Title: Details of the object model
on GitHub

No flaws! 🎉


URL: /en-US/docs/Web/JavaScript/Typed_arrays
Title: JavaScript typed arrays
on GitHub
Flaw count: 1

  • broken_links:
    • Can't resolve /en-US/docs/Code_snippets/StringView

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec
Title: RegExp.prototype.exec()
on GitHub

No flaws! 🎉


URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/Math
Title: Math
on GitHub

No flaws! 🎉


URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/log1p
Title: Math.log1p()
on GitHub
Flaw count: 1

  • bad_pre_tags:
    • <pre><code>CODE can be just <pre>CODE

URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply
Title: Function.prototype.apply()
on GitHub

No flaws! 🎉


URL: /en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties
Title: Enumerability and ownership of properties
on GitHub

No flaws! 🎉

External URLs

URL: /en-US/docs/Web/JavaScript/EventLoop
Title: Concurrency model and the event loop
on GitHub


URL: /en-US/docs/Web/JavaScript/Guide/Details_of_the_Object_Model
Title: Details of the object model
on GitHub


URL: /en-US/docs/Web/JavaScript/Typed_arrays
Title: JavaScript typed arrays
on GitHub


URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/RegExp/exec
Title: RegExp.prototype.exec()
on GitHub


URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/Math
Title: Math
on GitHub


URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/Math/log1p
Title: Math.log1p()
on GitHub


URL: /en-US/docs/Web/JavaScript/Reference/Global_Objects/Function/apply
Title: Function.prototype.apply()
on GitHub


URL: /en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties
Title: Enumerability and ownership of properties
on GitHub

No external URLs

Copy link
Member

@Elchi3 Elchi3 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, Will! Great work! Not sure what to think about the remaining one but I guess we will come back to it anyways when we look at how we want to do tables in GFM.

@Elchi3 Elchi3 merged commit 3a55456 into mdn:main Apr 15, 2021
@sideshowbarker
Copy link
Collaborator

The most involved case was developer.mozilla.org/en-US/docs/Web/JavaScript/Enumerability_and_ownership_of_properties, which uses inline styles to stop the layout from breaking when you have huge tables. I've reorganized and broken up the tables to avoid this (and I think the result is much clearer although I still find this page a bit baffling). I hope I didn't introduce any errors here, it's a bit intricate.

Nice work — it looks really great. I think what you came up with is a very big improvement in the usability of that content.

The case I didn't address was developer.mozilla.org/en-US/docs/Web/JavaScript/Equality_comparisons_and_sameness. One excuse is that the styles are all inside tables, and I expect that we will keep at least some tables in HTML even when we are in Markdown, so we don't need to fix them now. But perhaps I should try to attack it anyway, inline styles are not a good idea anyway.

Inspired by your work on the table in the “Enumerability and ownership of properties” article, I took a shot at re-working the nearly-as-ugly table in the “Loose equality using ==” section in the “Equality comparisons and sameness” article. The result is in #4133. (And in re-working the table, I took the opportunity to fix some other fairly fundamental problems with the content).

I didn’t touch the other table in that article — the one in the “A model for understanding equality comparisons?” section — which uses inline styles to color some of the cells with a green background, and other red. But I think we could just drop the style attributes from that table altogether. The green/red background stuff is nice, but it’s not essential.

@sideshowbarker
Copy link
Collaborator

I didn’t touch the other table in that article — the one in the “A model for understanding equality comparisons?” section — which uses inline styles to color some of the cells with a green background, and other red. But I think we could just drop the style attributes from that table altogether. The green/red background stuff is nice, but it’s not essential.

So I figured out a way to preserve some green/red indicators without needing CSS — which is, instead use the ✅ and ❌ emoji characters. So I opened #4151 with a patch that drops the inline styles and replaces them with those emoji.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Aug 17, 2022
@wbamberg wbamberg deleted the remove-js-inline-styles branch October 15, 2022 17:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants