atom / atom.io

🌐 A place for feedback on the atom.io website and package API
158 stars 98 forks source link

`All Changes` in Release Notes should be hidden by default #136

Open heinst opened 6 years ago

heinst commented 6 years ago

For the Release Notes page, there is a Caret that is enabled by default that says All Changes but it doesn't do anything. I am assuming that all the PRs should be under this and hidden by default. Not how it is currently

BinaryMuse commented 6 years ago

Hm, this appears to be a bug in however we're parsing the markdown data; as shown in this screenshot from the DOM inspector, the details tag does not fully surround the content like it does on https://github.com/atom/atom/releases.

cursor_and_release_notes

BinaryMuse commented 6 years ago

Okay, after playing around with it, I found a reproduction case: add two newlines after the </summary> tag, e.g.

<details>
<summary>Title</summary>
More stuff!
</details>

works but

<details>
<summary>Title</summary>

More stuff!
</details>

does not.

str1 = "<details>\n<summary>Title</summary>\nMore stuff!\n</details>"
str2 = "<details>\n<summary>Title</summary>\n\nMore stuff!\n</details>"
AtomIo::HTML.gfm(str1).to_s
# => "<p><details>\n<summary>Title</summary>\nMore stuff!\n</details></p>"
AtomIo::HTML.gfm(str2).to_s
# => "<p><details>\n<summary>Title</summary></details></p>\n\n<p>More stuff!\n</p>"

/cc @jasonrudolph for 💭

jasonrudolph commented 6 years ago

@heinst: Thanks for reporting this. :bow:

@BinaryMuse: Nice find. :zap: We have a handful of places where atom.io renders markdown differently than github.com. That almost always results in surprise and frustration :grimacing:, so I'd love to resolve those differences. /cc https://github.com/atom/atom.io/issues/121 https://github.com/atom/atom.io/issues/125 https://github.com/atom/atom.io/issues/130

The differences are mostly due to the fact that atom.io currently relies on html-pipeline for markdown rendering, while github.com has migrated away from html-pipeline in the move to CommonMark. We could likely find and resolve the specific glitch that's causing this issue with the way atom.io renders <summary> and <detail> tags, but it's largely a game of whack-a-mole at this point. The more robust solution might be to migrate atom.io to use github.com's current markdown-rendering tooling, or perhaps we could outsource markdown rendering to github.com altogether to avoid future drift between atom.io's markdown rendering and github.com's markdown rendering.