-
Notifications
You must be signed in to change notification settings - Fork 193
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
Update Style for Generating Reference Model Anchors at Build Time #1789
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Excellent work, just some important but very minor feedback to make the class names more legible to me. Also, I have some follow-up questions about layover AnchorJS styles we will be removing.
@JustKuzya, did you get a chance to updated PR feedback? We will wrap up the sprint today so I wanted to know if we can close this out for this sprint or not. |
I thought that I pushed it either Monday last week or Friday the week before. I probably need to map the changes to the Issue from github
DC
From: A.J. Stein ***@***.***>
Sent: Tuesday, May 30, 2023 9:44 AM
To: usnistgov/OSCAL ***@***.***>
Cc: Cousin, Dmitry A. (Fed) ***@***.***>; Mention ***@***.***>
Subject: Re: [usnistgov/OSCAL] Update Style for Generating Reference Model Anchors at Build Time (PR #1789)
@JustKuzya<https://github.com/JustKuzya>, did you get a chance to updated PR feedback? We will wrap up the sprint today so I wanted to know if we can close this out for this sprint or not.
-
Reply to this email directly, view it on GitHub<#1789 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/ACS6IVT6KWI3FR5YEH2N3RDXIX2QFANCNFSM6AAAAAAX6WL5H4>.
You are receiving this because you were mentioned.Message ID: ***@***.******@***.***>>
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The "hooked up" apparently is just one right-hand ring-finger flutter away from the "hooled up" :) My bad for not catching it before push
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@JustKuzya can you update the metaschema submodule to the current develop branch commit now that PR has been merged?
Bump on this @JustKuzya, because we need the submodule moved up before I can approve and merge this (or I wouldn't keep bringing it up). |
Sigh, how can we even get a merge conflict on pushing up a submodule!? I didn't notice this while pairing. I will have to investigate later. |
fec9021
to
1abab7d
Compare
The local performance metrics were dubious when run in a virtualized developer weird ways but Dmitry provided interim metrics (Site-Benchmarks.xlsx). Running |
…nistgov#1789) * isset Did not seem to work - changed it to ne (!= or <>) * Second part of the Anchor-JS unhooking and static Anchor Generated * hooled up the renamed class name .reference-element-anchor * Adding metaschema submodule changes (fixed with merge conflict).
…nistgov#1789) * isset Did not seem to work - changed it to ne (!= or <>) * Second part of the Anchor-JS unhooking and static Anchor Generated * hooled up the renamed class name .reference-element-anchor * Adding metaschema submodule changes (fixed with merge conflict).
Committer Notes
Partial fix for usnistgov/issue#1681
These two files hoos up anchors-js stuff for the older style deep-links at the recommended time in the event-lifecycle
Also makes sure that CSS hooks up behaviors to old site and ignores the newer versions.
"?
By submitting a pull request, you are agreeing to provide this contribution under the CC0 1.0 Universal public domain dedication.
(For reviewers: The wiki has guidance on code review and overall issue review for completeness.)
Changes to Core Features:
Have you written new tests for your core changes, as applicable?Have you included examples of how to use your new feature(s)?