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

Linked headlines #3540

Merged
merged 25 commits into from
Jun 23, 2023
Merged

Linked headlines #3540

merged 25 commits into from
Jun 23, 2023

Conversation

tiberiuichim
Copy link
Contributor

No description provided.

@netlify
Copy link

netlify bot commented Jul 31, 2022

Deploy Preview for volto ready!

Name Link
🔨 Latest commit 9d4e659
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/64957c7ac8a5ed0008196a41
😎 Deploy Preview https://deploy-preview-3540--volto.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@cypress
Copy link

cypress bot commented Jul 31, 2022

Passing run #5860 ↗︎

0 507 20 0 Flakiness 0

Details:

Linked headlines refactor (#4287)
Project: Volto Commit: 9d4e6590fa
Status: Passed Duration: 14:10 💡
Started: Jun 23, 2023 11:08 AM Ended: Jun 23, 2023 11:22 AM

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings.

@tiberiuichim
Copy link
Contributor Author

@sneridagh This works, but the cypress and a11y tests need to be fixed. I'll need assistance on this, also maybe the CSS needs to be redone.

@avoinea
Copy link
Member

avoinea commented Jan 17, 2023

@tiberiuichim

Hmm. This breaks the Table of contents block as it is generated based on the blocks uids.

Also, @razvanMiu confirmed that the block uids are used somewhere within the Link widget to link to sections within the page.

Thus, I think it's safer for now to go with the block UIDs in order not to introduce it as a breaking change and in a separate PR/PLIP we can talk about changing the heading ids to more friendly names (maybe the same way we do with the Override the TOC entry checkbox).

@tiberiuichim
Copy link
Contributor Author

"breaking the toc" is not a blocker, we can fix the toc block.

It would represent a major breaking change for Volto, though.

@davisagli davisagli added this to the 17.x.x milestone Jan 23, 2023
@tisto
Copy link
Member

tisto commented Mar 6, 2023

@tiberiuichim I checked out this branch and I quite like it! I guess we always thought too hard about how an editor could enter the anchor ID in the UI. Though, this solution is both elegant and simple. The only possible downside that I see is that links can break easily when editors change a headline.

@avoinea
Copy link
Member

avoinea commented Mar 6, 2023

@tisto This feature is also available as an add-on until it hits Volto Core. See: https://github.com/eea/volto-anchors

@nileshgulia1
Copy link
Member

I was also thinking about the adaptability of this feature, whether make it available per content-type, or per block?

@tiberiuichim
Copy link
Contributor Author

The only possible downside that I see is that links can break easily when editors change a headline.

Yes, it's a downside. But I would argue that we shouldn't focus too much on this type of scenario, it's counterproductive. Honestly, because this PR is modeled after the github readmes, it happened to me many times that I arive at a document where the link is no longer valid. And it's frustrating, yes, but in reality I could scan the destination document to find the info I was looking for. Or I could look at the URL fragment and understand what the new section would be. Or the document was fully refactored, split into multiple pages, so the link would no longer be valid anyway.

What we're gaining is simplicity for the "happy case".

@tisto
Copy link
Member

tisto commented Mar 11, 2023

@tiberiuichim would it be possible to enable/disable this via a feature toggle? This would give us some flexibility in which Volto version we could ship this feature.

@tiberiuichim
Copy link
Contributor Author

@tisto sure!

Even in its current form, it's ultimately configurable, as we're replacing the way the hx elements are rendered, in the config:

image

@tisto
Copy link
Member

tisto commented Mar 12, 2023

@tiberiuichim awesome!

@sneridagh sneridagh merged commit e2ed2fc into master Jun 23, 2023
@sneridagh sneridagh deleted the linked-headlines branch June 23, 2023 12:18
@nileshgulia1
Copy link
Member

🎉

sneridagh added a commit that referenced this pull request Jun 25, 2023
* master:
  Release 17.0.0-alpha.14
  Linked headlines (#3540)
  Release notes for 16.20.8 16.21.0 16.21.1 (#4910)
  Spanish translation (#4896)
  Refactor Anontools (#4845)
  Update to plone-backend 6.0.5 (#4897)
  Release 17.0.0-alpha.13
  Enforce max upload size (#4868)
  Fix and improve the `addStyling` helper (#4880)
  Release 17.0.0-alpha.12
  Fix regression in horizontal scroll in contents view, add it back (#4872)
  Configurable Container component from registry for some key route views. (#4871)
  Allow to deselect color in ColorPickerWidget. (#4839)
  Release 17.0.0-alpha.11
  Pagination with router params (#4698)
  Release 17.0.0-alpha.10
  feat(slate): Add css identifier to slate style menu options (#4847)
  Update Brazilian Portuguese translations (Fixes #4853)
  Convert header class to function (#4767)
sneridagh added a commit that referenced this pull request Jun 26, 2023
* master: (29 commits)
  Remove anonymous function calls. Remove default exports from. (#4917)
  Release 17.0.0-alpha.14
  Linked headlines (#3540)
  Release notes for 16.20.8 16.21.0 16.21.1 (#4910)
  Spanish translation (#4896)
  Refactor Anontools (#4845)
  Update to plone-backend 6.0.5 (#4897)
  Release 17.0.0-alpha.13
  Enforce max upload size (#4868)
  Fix and improve the `addStyling` helper (#4880)
  Release 17.0.0-alpha.12
  Fix regression in horizontal scroll in contents view, add it back (#4872)
  Configurable Container component from registry for some key route views. (#4871)
  Allow to deselect color in ColorPickerWidget. (#4839)
  Release 17.0.0-alpha.11
  Pagination with router params (#4698)
  Release 17.0.0-alpha.10
  feat(slate): Add css identifier to slate style menu options (#4847)
  Update Brazilian Portuguese translations (Fixes #4853)
  Convert header class to function (#4767)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants