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

use "<a></a>Content" instead of "<a>Content</a>" #818

Merged
merged 1 commit into from
Jan 16, 2024
Merged

use "<a></a>Content" instead of "<a>Content</a>" #818

merged 1 commit into from
Jan 16, 2024

Conversation

jcbhmr
Copy link
Contributor

@jcbhmr jcbhmr commented Jan 16, 2024

Why this is a good idea:

  • Makes it much easier to apply classless CSS to the resulting HTML
  • Conform with ecosystem convention

All of these classless CSS frameworks style all <a> tags the same. https://github.com/dbohdan/classless-css

ALL OF THEM treat <a> as "a link" even if it has no href. This is usually what you want since you might apply an onclick that you want to appear as a phantom link. This isn't the case with <a name="..."> in the generated HTML from wit-bindgen markdown. I suggest instead following a precedent of empty <a name="..."> tags.

This <a name=""></a> thing has been done before. Quite a lot actually. https://github.com/search?q=%2F%3Ca%20name%3D%5B%22%27%5D.%2B%5B%22%27%5D%3E%3C%5C%2Fa%3E%2F&type=code

Consider:

Before:
image

After:
image

Imagine a whole page full of phantom links and you can see why I want to make this change.

This would still preserve the page.html#the-thing of the <a name="the-thing"></a> while making it compatible with the vast majority of CSS that already exists in the ecosystem of classless whole-page CSS presets.

Yes, you can solve this by using :any-link { ... } https://developer.mozilla.org/en-US/docs/Web/CSS/:any-link but realistically I'm not going to open 100+ PRs on projects that are one-and-done and don't get any more attention -- CSS presets rarely seem to get updated after they are pretty enough. It's much easier just to tweak the generation here to conform to ecosystem expectations.

Alternatives:

  • Add custom CSS to the generated HTML that accounts for this. This could be as basic as forking one of the existing classless CSS projects and tweaking it and then using the resulting style.css in the generated HTML. But that's more in wit-doc territory, not wit-bindgen markdown.

Copy link
Contributor

@pchickey pchickey left a comment

Choose a reason for hiding this comment

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

Thanks for the fix and the explanation.

@pchickey pchickey merged commit 7c9c462 into bytecodealliance:main Jan 16, 2024
9 checks passed
@jcbhmr jcbhmr deleted the patch-1 branch January 16, 2024 22:11
lachieh added a commit to lachieh/wit-bindgen that referenced this pull request Jul 23, 2024
lachieh added a commit to lachieh/wit-bindgen that referenced this pull request Jul 23, 2024
…iance#818

Signed-off-by: Lachlan Heywood <lachieh@users.noreply.github.com>
github-merge-queue bot pushed a commit that referenced this pull request Jul 23, 2024
Signed-off-by: Lachlan Heywood <lachieh@users.noreply.github.com>
cpetig pushed a commit to cpetig/wit-bindgen that referenced this pull request Jul 25, 2024
…iance#818 (bytecodealliance#1008)

Signed-off-by: Lachlan Heywood <lachieh@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants