Linked headlines refactor #4287

nileshgulia1 · 2023-01-20T12:11:00Z

Use <UniveralLink/> in linked heading anchor since we are now using Links generated by react-router-hashlink.

netlify · 2023-01-20T12:11:06Z

✅ Deploy Preview for volto canceled.

Name	Link
🔨 Latest commit	`b787548`
🔍 Latest deploy log	https://app.netlify.com/sites/volto/deploys/648ded896d3e8800088698f2

cypress · 2023-01-20T13:50:05Z

Passing run #5717 ↗︎

0	507	20	0	0

Details:

refactor: move useClipboard hook to volto/src/hooks
Project: Volto	Commit: `b787548693`
Status: Passed	Duration: 12:31 💡
Started: Jun 17, 2023 5:32 PM	Ended: Jun 17, 2023 5:45 PM

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

… in hashlinks

nileshgulia1 · 2023-03-20T10:22:48Z

This just needs a towncrier news item, can be added collectively in #3540.
@sneridagh @tiberiuichim safe to merge?

nileshgulia1 · 2023-03-20T10:49:39Z

@ksuess I remember you asked for moving the "slug icon" to the right just like mdn docs. Would you mind reviewing this PR then?

ksuess

This is great, I would take it as it is.
But an opt-out (no chain icon on hover) would be handsome.

src/helpers/ScrollToTop/ScrollToTop.jsx

src/components/manage/Blocks/Title/View.jsx

ksuess · 2023-03-21T10:42:17Z

packages/volto-slate/src/editor/less/slate.less

      transform: rotate(45deg);
+      visibility: hidden;
    }
  }
 }


I would prefer a link icon aligned and with same size of the headline or just slightly increased in size.

Cosmetics: the link icon is rotated. Even if the shape of the icon is unusual, it's a global design decission.

@ksuess it's relatively difficult to make the link icon properly wrap with the text, so if it's multiple lines, it doesn't behave well. But my css knowledge may be limited, maybe there's a hack to do it

ksuess · 2023-03-21T10:47:52Z

packages/volto-slate/src/editor/render.jsx

+            aria-hidden="true"
+            tabIndex={-1}
+            href={`#${slug}`}
+          >
            <svg
              {...linkSVG.attributes}
              dangerouslySetInnerHTML={{ __html: linkSVG.content }}
              width="2em"


overriden by less CSS: "width="2em""

nileshgulia1 · 2023-03-22T07:46:41Z

But an opt-out (no chain icon on hover) would be handsome.

"no chain icon on hover" - hm.. I don't see a reason why users will don't need it. @ksuess Could you give an example of sites where there is no link icon on hover but are anchors. Maybe I can just make that icon configurable? So a "#" in place of a chain icon.

ksuess · 2023-03-22T08:26:01Z

But an opt-out (no chain icon on hover) would be handsome.

"no chain icon on hover" - hm.. I don't see a reason why users will don't need it. @ksuess Could you give an example of sites where there is no link icon on hover but are anchors. Maybe I can just make that icon configurable? So a "#" in place of a chain icon.

Sorry, what I mean instead is: In my opinion, "linked headlines" should be optional. Or at least opting out should be possible.

I understand that this topic should go in a main issue, but I am confused where the current main development happens. Feel free to move this comment to where it belongs, thank you.

sneridagh · 2023-03-22T09:12:15Z

I also think it should be optional, not sure if "opt-in" or "opt-out"... I can certainly think it sites that do not need it (brochure, etc).

nileshgulia1 · 2023-05-18T12:21:53Z

@sneridagh Now that I started to implement this, I'm a bit puzzled having both "copy to clipboard" and "scroll to anchor" events triggering on clicking anchor icon. @albertcasado when an anchor is at the bottom of the page, clicking on it scrolls to that anchor. Could triggering a toast simultaneously annoy a user? 🤔

nileshgulia1 · 2023-06-07T17:37:01Z

@sneridagh I have pushed the changelog. Would you mind reviewing it, please?

stevepiercy

I made a few suggestions and questions for your consideration.

news/4287.feature

src/helpers/MessageLabels/MessageLabels.js

docs/source/configuration/volto-slate/configuration-settings.md

nileshgulia1 · 2023-06-09T15:10:40Z

@stevepiercy I resolved your comments in de9e57f. Could you please have a look? Thanks!

nileshgulia1 · 2023-06-09T15:23:51Z

@sneridagh Now that I started to implement this, I'm a bit puzzled having both "copy to clipboard" and "scroll to anchor" events triggering on clicking anchor icon. @albertcasado when an anchor is at the bottom of the page, clicking on it scrolls to that anchor. Could triggering a toast simultaneously annoy a user? 🤔

@stevepiercy @sneridagh Anyways, what do you think of this opinion, As a user? Can we disable toast and instead show a copied "SVG" near the anchor.

Current scenario

Screen.Recording.2023-06-09.at.8.57.08.PM.mov

stevepiercy · 2023-06-10T06:44:55Z

@sneridagh Now that I started to implement this, I'm a bit puzzled having both "copy to clipboard" and "scroll to anchor" events triggering on clicking anchor icon. @albertcasado when an anchor is at the bottom of the page, clicking on it scrolls to that anchor. Could triggering a toast simultaneously annoy a user? 🤔

@stevepiercy @sneridagh Anyways, what do you think of this opinion, As a user? Can we disable toast and instead show a copied "SVG" near the anchor.

Current scenario

Screen.Recording.2023-06-09.at.8.57.08.PM.mov

For code blocks in docs, we have this interface, which is nice. It has both the universal icon for copy, plus translatable text. You can play with it at the permalink:

https://6.docs.plone.org/volto/configuration/how-to.html#extending-configuration-in-a-project

Screen.Recording.2023-06-09.at.11.41.29.PM.mov

I think the toast at the bottom is too much and disconnected from the click on large screens.

stevepiercy

I approve docs and naming conventions. A maintainer should review source code, too. Thank you!

nileshgulia1 · 2023-06-12T11:04:27Z

Thank you @stevepiercy .

davisagli

I took a look and I like it! I do have a few suggestions...

Should we close #3540 and edit this PR to set the target branch to master? I don't think merging it into #3540 first has any particular benefit.

packages/volto-slate/src/editor/render.jsx

src/components/manage/Blocks/ToC/variations/DefaultTocRenderer.jsx

packages/volto-slate/src/editor/less/slate.less

src/components/manage/Blocks/Title/View.jsx

davisagli · 2023-06-13T05:35:19Z

Anyways, what do you think of this opinion, As a user? Can we disable toast and instead show a copied "SVG" near the anchor.

@nileshgulia1 I think it would be nice to have it closer to where the user clicks, but I am not sure how to guarantee that there is space to show it. The toast is okay and has the benefit of (hopefully) following existing practices for a11y rather than needing to make sure something new is accessible...I would keep the toast for now and come back to it as a future improvement.

packages/volto-slate/src/hooks/useClipboard.jsx

davisagli

LGTM now

nileshgulia1 added 3 commits January 20, 2023 14:18

remove gh-slugger: use block uids

86ece05

Merge branch 'linked-headlines' into linkedHeadlinesRefactor

e8c753c

fix scrollToTop conflict, use universalLink in linked anchor

e355aec

fix cypress

4d34b4e

nileshgulia1 mentioned this pull request Jan 22, 2023

Slug based linked headlines in volto-slate #4298

Closed

8 tasks

nileshgulia1 and others added 12 commits January 24, 2023 15:28

support anchor links in title block,always show link icon in mobile

ab0dd7d

remove rest props

5e2b535

add server condition in scrollToTop to prevent calling multiple times…

bf40ad5

… in hashlinks

add back github-slugger

4665dac

make title block anchor sluggified

f9db2bf

adapt slug based scroll in TOC blocks

1d006f8

switch position of anchor linkSVG

4296701

(jest): fix snapshot

f37ac59

fix cypress

915df5c

Merge branch 'linked-headlines' into linkedHeadlinesRefactor

0c53617

cypress: fix guillotina tests

0f97fd3

Merge branch 'linked-headlines' into linkedHeadlinesRefactor

878d7cd

avoinea requested a review from tiberiuichim March 7, 2023 21:15

Merge branch 'linked-headlines' into linkedHeadlinesRefactor

a7da5d4

nileshgulia1 requested a review from ksuess March 20, 2023 10:50

ksuess requested changes Mar 21, 2023

View reviewed changes

nileshgulia1 added 2 commits April 18, 2023 11:34

Merge branch 'linked-headlines' into linkedHeadlinesRefactor

401fba9

anchor animation styles

757b389

nileshgulia1 added 2 commits May 18, 2023 17:05

fix(code-analysis): run prettier

a8e4fab

feat: add docs for linkedHeadlines

8b8c073

nileshgulia1 added 5 commits May 19, 2023 12:52

fix(tests): update snapshots

6f7436c

run yarn i18n

26bd444

feat: add cypress test for anchors-headings

bdd8762

Merge branch 'linked-headlines' into linkedHeadlinesRefactor

51cf4f1

chore: update changelog, run i18n

a39122e

nileshgulia1 requested a review from stevepiercy June 7, 2023 15:58

stevepiercy requested changes Jun 7, 2023

View reviewed changes

nileshgulia1 added 2 commits June 9, 2023 12:39

refactor: use headings instead of headlines, resolved comments

de9e57f

chore: yarn i18n

8d20016

stevepiercy approved these changes Jun 10, 2023

View reviewed changes

davisagli requested changes Jun 13, 2023

View reviewed changes

nileshgulia1 added 2 commits June 16, 2023 17:24

refactor: support for listing headlines,css and TOC fixes

c33f634

fix: update jest snapshots

4a062a5

tiberiuichim reviewed Jun 16, 2023

View reviewed changes

packages/volto-slate/src/hooks/useClipboard.jsx Outdated Show resolved Hide resolved

nileshgulia1 added 2 commits June 17, 2023 22:16

Merge branch 'linked-headlines' into linkedHeadlinesRefactor

11192a5

refactor: move useClipboard hook to volto/src/hooks

b787548

davisagli approved these changes Jun 18, 2023

View reviewed changes

sneridagh approved these changes Jun 23, 2023

View reviewed changes

sneridagh merged commit 9d4e659 into linked-headlines Jun 23, 2023

sneridagh deleted the linkedHeadlinesRefactor branch June 23, 2023 11:05

cypress bot mentioned this pull request Jun 23, 2023

Linked headlines #3540

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Linked headlines refactor #4287

Linked headlines refactor #4287

nileshgulia1 commented Jan 20, 2023 •

edited

Loading

netlify bot commented Jan 20, 2023 •

edited

Loading

cypress bot commented Jan 20, 2023 •

edited

Loading

nileshgulia1 commented Mar 20, 2023

nileshgulia1 commented Mar 20, 2023

ksuess left a comment

ksuess Mar 21, 2023 •

edited

Loading

tiberiuichim Mar 21, 2023

ksuess Mar 21, 2023 •

edited

Loading

nileshgulia1 commented Mar 22, 2023

ksuess commented Mar 22, 2023

sneridagh commented Mar 22, 2023

nileshgulia1 commented May 18, 2023

nileshgulia1 commented Jun 7, 2023

stevepiercy left a comment

nileshgulia1 commented Jun 9, 2023

nileshgulia1 commented Jun 9, 2023 •

edited

Loading

stevepiercy commented Jun 10, 2023

stevepiercy left a comment

nileshgulia1 commented Jun 12, 2023

davisagli left a comment

davisagli commented Jun 13, 2023

davisagli left a comment

Linked headlines refactor #4287

Linked headlines refactor #4287

Conversation

nileshgulia1 commented Jan 20, 2023 • edited Loading

netlify bot commented Jan 20, 2023 • edited Loading

✅ Deploy Preview for volto canceled.

cypress bot commented Jan 20, 2023 • edited Loading

Passing run #5717 ↗︎

nileshgulia1 commented Mar 20, 2023

nileshgulia1 commented Mar 20, 2023

ksuess left a comment

Choose a reason for hiding this comment

ksuess Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

tiberiuichim Mar 21, 2023

Choose a reason for hiding this comment

ksuess Mar 21, 2023 • edited Loading

Choose a reason for hiding this comment

nileshgulia1 commented Mar 22, 2023

ksuess commented Mar 22, 2023

sneridagh commented Mar 22, 2023

nileshgulia1 commented May 18, 2023

nileshgulia1 commented Jun 7, 2023

stevepiercy left a comment

Choose a reason for hiding this comment

nileshgulia1 commented Jun 9, 2023

nileshgulia1 commented Jun 9, 2023 • edited Loading

stevepiercy commented Jun 10, 2023

stevepiercy left a comment

Choose a reason for hiding this comment

nileshgulia1 commented Jun 12, 2023

davisagli left a comment

Choose a reason for hiding this comment

davisagli commented Jun 13, 2023

davisagli left a comment

Choose a reason for hiding this comment

nileshgulia1 commented Jan 20, 2023 •

edited

Loading

netlify bot commented Jan 20, 2023 •

edited

Loading

cypress bot commented Jan 20, 2023 •

edited

Loading

ksuess Mar 21, 2023 •

edited

Loading

ksuess Mar 21, 2023 •

edited

Loading

nileshgulia1 commented Jun 9, 2023 •

edited

Loading