-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Improve documentation sidesheet #9724
Conversation
const { data: docs, isLoading } = useDocumentation(documentationUrl); | ||
|
||
const removeBaseUrl = (url: { path: string }) => { | ||
if (url.path.match(/^..\/..\/.*/)) { |
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.
❓ Could you give an example of what URLs we're trying to rewrite here? This currently matches everything beginning with two arbitrary letters than slash then two more arbitrary letters? My feeling of gut is we're trying to rewrite specifically ../../
URLs here (in which case the dots would need to be escaped in the regex). Also if that's the case we could improve this by simply using startsWith
, which is a bit more performant, and easier to read than /^\.\.\/\.\.\/.*
:)
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.
Good catch! Fixed
@@ -49,6 +49,14 @@ task copyDocs(type: Copy) { | |||
duplicatesStrategy DuplicatesStrategy.INCLUDE | |||
} | |||
|
|||
task copyAssets(type: Copy) { |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
What
Fixes #8411
Update styles for Documentation Setup Guide sidesheet
Remove html comments
Improve tables
Use colors from app theme
Fix font for titles (h1, h2) and bold text
Improve links