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

Anchor link plugin bugs/issues #3870

Closed
natebyerley opened this issue Nov 6, 2024 · 3 comments · Fixed by #3872
Closed

Anchor link plugin bugs/issues #3870

natebyerley opened this issue Nov 6, 2024 · 3 comments · Fixed by #3872
Assignees
Labels
2.12.x only bug Something isn't working

Comments

@natebyerley
Copy link

natebyerley commented Nov 6, 2024

Problem/Motivation

Issues discovered when testing this P/R: #3219

Describe the bug

  • anchor links added using the CK editor add an <a> tag and append an id to the <a> element (rather than modifying the existing element (i.e. <p> or <h> tag
  • anchor links added using the CK editor also add the following class="ck-anchor" to the <a> element, which doesn't appear to have the appropriate styles
  • anchor links added using the CK editor appear red/underlined/hyperlinked on front end, and pointer cursor is not consistent with other links on site
  • possible to add an anchor link to text that already has an anchor link
  • possible to add multiple duplicate anchor links to a page (front end user will land on top-most anchor link element if there are multiple on a page)

To Reproduce

Examples:

https://212alpha1-azs-housing.pantheonsite.io/test#h2

https://212alpha1-azs-housing.pantheonsite.io/test#p

Inspecting the elements shows:

Screenshot 2024-11-06 at 10 49 55 AM

Proposed resolution

Style issues

  • Inherit styles for class "ck-anchor"? This would allow uniquely styled anchor link text to remain styled with an anchor attached.
  • Is it accessibility best practice to add id to an <a> element?

Duplicate anchor link issues

  • This will likely be hard to fix/change. We may just need to let users know that each page should only have unique anchor links.
  • Adding anchor links to text that already has anchor links in source might create confusion among users, but is easily revealed by viewing source. Again, this might best be addressed through documentation.

Expected behavior

  • Adding an anchor link should not alter front-end style of anchor link text
  • Add documentation that covers limitations of this feature (it does not verify if your anchor link is unique)

Additional context

Slack discussion:

https://uarizona.slack.com/archives/C9EDC2RDM/p1730752456244879

@danahertzberg
Copy link
Contributor

@accesswatch did confirm the ID can be on any type of element. <a> is not required.

@bberndt-uaz bberndt-uaz self-assigned this Nov 6, 2024
@bberndt-uaz
Copy link
Contributor

bberndt-uaz commented Nov 6, 2024

I came across this issue with the CKEditor Anchor Link module:

I confirmed that IDs could be added to <a> elements in 2.11 and that this is no longer possible with the main Quickstart branch. Since existing <a> elements with IDs could have those IDs deleted upon editing a page, we can consider this a regression with the potential for data loss. After discussion in the AZ Digital workshop today, we are proposing to revert our addition of the CKEditor Anchor Link module to Quickstart. Instead, we will create a new Quickstart issue with our requirements for an anchor links feature. We can also track the progress of the CKEditor Bookmarks initiative.

@bberndt-uaz
Copy link
Contributor

Rather than creating a new issue about adding anchor link functionality to Quickstart, we have updated this existing issue:

This issue (#3870) will remain to document what we have found with the current implementation in our 2.12 pre-release, which uses the CKEditor Anchor Link module. We are still proposing to revert our addition of this module to Quickstart.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2.12.x only bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants