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

Remove fragment-identifier package #71

Merged
merged 2 commits into from
Apr 3, 2020
Merged

Conversation

tilgovi
Copy link
Contributor

@tilgovi tilgovi commented Apr 3, 2020

Due to unresolved questions about the fragment identifier format and
difficulties around parsing the fragment identifier correctly, remove
the fragment-identifier package entirely for the time being.

See also w3c/web-annotation#443.

Close #66.

Copy link
Contributor

@Treora Treora left a comment

Choose a reason for hiding this comment

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

I pushed my suggestions as an additional commit, seemed easier than commenting.

@Treora
Copy link
Contributor

Treora commented Apr 3, 2020

And fantastic that you made work of this right away!

I did however plan to first merge the tests and tweaks I did yesterday, in order to keep them available in our history (without having a dead branch linger around forever).

I will just merge those additions first, then we can remove them again while merging this. :)

tilgovi and others added 2 commits April 3, 2020 15:02
Due to unresolved questions about the fragment identifier format and
difficulties around parsing the fragment identifier correctly, remove
the fragment-identifier package entirely for the time being.

See also [w3c/web-annotation#443].

Close #66.

[w3c/web-annotation#443]: w3c/web-annotation#443
- No need to run upon page load.
- In selectionChange handler, test if selector has a value
- Rename refresh→anchor (call cleanup() separately)
- Make element–example relation less magic
@Treora Treora force-pushed the remove-fragment-identifier branch from 25a4a66 to 6dfe24d Compare April 3, 2020 13:04
@Treora
Copy link
Contributor

Treora commented Apr 3, 2020

To resolve merge conflicts already, I rebased onto master and force-pushed this PR. Hope that’s okay!

@tilgovi
Copy link
Contributor Author

tilgovi commented Apr 3, 2020

Excellent! Thank you.

@tilgovi tilgovi merged commit d86fe58 into master Apr 3, 2020
@tilgovi tilgovi deleted the remove-fragment-identifier branch April 3, 2020 14:50
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