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

Dfn panel changes for linking syntax #2685

Merged
merged 6 commits into from
Oct 4, 2023
Merged

Conversation

dlaliberte
Copy link
Collaborator

@dlaliberte dlaliberte commented Oct 3, 2023

This PR adds a "Possible linking syntaxe(es)" section to dfn panels.

  • The copy icon is perhaps overkill.
  • The cursor for the dfn is changed to be a 'help' cursor, since the dfn is not a link to anything but the dfn panel.
  • There are also a couple css changes regarding dfn panels in general, to maybe fix some whitespace issues, depending on what was intended.
    • Avoid wrapping after section numbers.
    • No ellipsis needed most of the time.
    • Multiple reference numbers no longer wrap (e.g. (2) (3) (4))

Here is what it looks like now.

image

@@ -226,7 +312,6 @@
pinDfnPanel(dfnPanel);
}
event.stopPropagation();
event.preventDefault();
Copy link
Collaborator Author

@dlaliberte dlaliberte Oct 4, 2023

Choose a reason for hiding this comment

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

Without event.preventDefault(), the page scrolls so that the dfn is at the top of the window, and the dfn panel appears to just disappear, but it is moved simultaneously to the bottom left corner. If that is your intention, fine. I found it confusing, however. I would guess you intend this behavior to make it easy to revisit the dfn by clicking the header link in the previously activated dfn panel. I so, maybe we should think about doing that and more in a better way.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I hadn't realized that this event.preventDefault() call was also disabling all the other links on the panel. We'll look into fixing the scrolling behavior perhaps a different way.

@dlaliberte dlaliberte merged commit 1d55232 into main Oct 4, 2023
31 checks passed
@dlaliberte dlaliberte deleted the dlaliberte-dfnpanels-09-20 branch October 4, 2023 18:17
@dlaliberte
Copy link
Collaborator Author

This PR fixes #1319

This pull request was closed.
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