-
Notifications
You must be signed in to change notification settings - Fork 30
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
Docs UI update #3
Conversation
@ZYJLiu is attempting to deploy a commit to the Solana Foundation Team on Vercel. A member of the Team first needs to authorize it. |
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
great work here John! the new components will be a great addition to the site. better UX all around! :)
I just have the one concern about the regex fix for other languages. we need to find a different solution
src/utils/contentApi.ts
Outdated
@@ -353,7 +353,7 @@ export default class ContentApi { | |||
|
|||
// we remove any special characters from the end of the label | |||
// to make it look cleaner in the UI | |||
label = label.split(/[\v{L|N}]*$/)[0]; | |||
label = label.replace(/[^a-zA-Z0-9]+$/, ""); |
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.
updating this regex to your suggesest change will break all translated content within the table of contents. and maybe prevent the site from building.
we need to find a different solution to solve the "trailing L" bug we are seeing
src/utils/contentApi.ts
Outdated
@@ -388,7 +388,7 @@ export default class ContentApi { | |||
.trim() | |||
.replace(/\s+/g, "-") | |||
.replace(/\./g, "") | |||
.replace(/[[\v{L|N}]*]+/g, "-") | |||
.replace(/[^a-z0-9]+/g, "-") |
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.
same comment as above
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.
Please add the usage guidelines for the new components (you've already added them in the text of this PR) into CONTRIBUTING.md
will add to developer-content, since that is where the components are used |
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.
Great work on this John. I think people are going to really like these new UX improvements
captured and will be accomplished in the dev content repo
added tracking issue on dev content: solana-foundation/developer-content#288 |
@@ -353,7 +353,7 @@ export default class ContentApi { | |||
|
|||
// we remove any special characters from the end of the label | |||
// to make it look cleaner in the UI | |||
label = label.replace(/[^a-zA-Z0-9]+$/, ""); | |||
label = label.replace(/[^\p{L}\p{N}]+$/u, ""); |
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.
BTW @ZYJLiu you can use \p{Letter}
and \p{Number}
to make the unicode bits more obvious. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Regular_expressions/Unicode_character_class_escape
Example usage on developer-content md files:
Example usage on developer-content md files:
Update docs codeblock UI with svgs icons in header
Add lightmode color to docs components/codeblock
Update docs scrollbar to only appear on hover
Fix doc table of content bug where "SOL" gets converted to "SO"
Update Table of Contents "Edit Page" and "Scroll to Top" buttons
Add Stack Exchange to docs secondary nav