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

Add the block id as an id attribute to all rendered blocks #8

Merged
merged 9 commits into from
Nov 28, 2021

Conversation

Mathspy
Copy link
Owner

@Mathspy Mathspy commented Nov 28, 2021

The decision to remove the dashes from the id is based on

  1. They kind of give an unrealistic expectancy that the different parts have different meaning or different relevancy which is not true for this use case (where they will mainly be used as fragment links #28c719a398454f089e871fe78e50e92b)
  2. When used for the fragment of the URL (i.e #28c719a3-9845-4f08-9e87-1fe78e50e92b) selecting the whole thing becomes a pain due to browsers considering dashes as word breaks. So double clicking selects only the segment between the two dashes instead of the whole thing.
  3. Notion already does it! If there's a link to a block in Notion, the API responds with the href looks like "/46f8638c25a84ccd9d926e42bdb5535e#28c719a398454f089e871fe78e50e92b" AKA /PAGE_ID_WITHOUT_DASHES#BLOCK_ID_WITHOUT_DASHES

The decision to link to the wrapper div instead of the paragraph itself
was made because currently when a link to a paragraph which has
children is taken on Notion, Notion highlights both the paragraph and
its children.
I went back and forth on this decision however so if linking to the
paragraph directly proves more useful we should go for it. Linking to
paragraphs directly feels more intuitive to me anyway
The decision to put the id on the list items instead of the lists
themselves is a fairly obvious one to me. No idea how would one put
them on the lists anyway
I put the id on the figure instead of the img tag because I think they
act as one entity and linking to the whole thing makes most sense
This is as opposed to lists where each item acts independently
Same as images here, the whole thing is one unified thing
@Mathspy Mathspy merged commit daa9f8f into main Nov 28, 2021
@Mathspy Mathspy deleted the id-everything branch November 28, 2021 19:32
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.

1 participant