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

[flutter_local_notifications] Fix table of contents in README #1562

Merged
merged 2 commits into from
May 1, 2022

Conversation

HendrikF
Copy link
Contributor

This fixes the table of contents at https://pub.dev/packages/flutter_local_notifications .

Android and iOS Setup do not work as the emoji breaks the link fragment.

Unfortunately I just realized, that GitHub seems to handle the #️Number Sign emoji differently, so it currently only works on GitHub.
With this fix it would work on pub.dev, but not on GitHub. 😐

@MaikuB
Copy link
Owner

MaikuB commented Apr 16, 2022

Thanks for the PR and picking up on this. Maybe it's a GitHub issue but I'm having trouble making out what the changes are. Are you able to elaborate on what exactly changed. Here's what I see on GitHub so to my untrained eye, it looks the same though GitHub but there must be a difference if GitHub picked up on it

image

How did you validate it worked on pub.dev too? Am I also right in saying that getting rid of emojis would completely get rid of the issue? I would assume so as emojis was a contibrution from @psyanite and I hadn't received reports of issues besides incorrect links prior to their addition. If this is causing issues then I would say it's better to remove the emojis so that the community can actually use the table of contents on both pub.dev and GitHub

@HendrikF
Copy link
Contributor Author

Oh now that's interesting. This is what I am seeing:
Bildschirmfoto von 2022-04-18 14-05-36

I also see emojis using developer tools on pub.dev:
Bildschirmfoto von 2022-04-18 14-06-59
If I change the href using devtools, the links in the toc start working.

I am using Firefox 99 on Ubuntu 20.04.4 LTS.

@MaikuB
Copy link
Owner

MaikuB commented Apr 23, 2022

Hmm so it was a browser issue. I was using Safari and also checked Chrome on macOS. Given the inconsistent behaviour, do you think you'd be able to help look into either removing the emojis or seeing if there's a different set that would be suitable but would work on pub.dev and GitHub?

@HendrikF
Copy link
Contributor Author

HendrikF commented Apr 27, 2022

I don't understand why GitHub is generating the # emoji within the link fragment. As it only happens for headings starting with the gear emoji, maybe we should just change that gear emoji? That way the two links would work on both platforms.

@MaikuB
Copy link
Owner

MaikuB commented Apr 29, 2022

Sure that was one of my suggestions so if you're able to find a suitable emoji and update the PR with it then I'd be happy to merge it in

@HendrikF
Copy link
Contributor Author

I swapped the gear for a wrench emoji. Now it works on GitHub and I don't see how it should not work on pub.dev.
You can test it here: https://github.com/HendrikF/flutter_local_notifications/tree/master/flutter_local_notifications

@HendrikF
Copy link
Contributor Author

I also opened a bug report with GitHub.

@MaikuB
Copy link
Owner

MaikuB commented May 1, 2022

Thanks for this. Hopefully, pub.dev won't be the one with an issue with this :D

@MaikuB MaikuB merged commit 659508f into MaikuB:master May 1, 2022
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