-
Notifications
You must be signed in to change notification settings - Fork 52
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
[SR-15313] [Swift-DocC-Render] Primary tutorial navigation dropdown fails to bold the current tutorial when browser URL is not lowercased #282
Comments
It seems like to be the url is not match so the isActive property(option.active) is not set. Then the "font-weight: 600" is not applied properly. So what do you think the following solutions? @mportiz08
|
@Kyle-Ye I think the first option of making a case insensitive comparison would be preferable to redirecting and enforcing a specific case style in the URL. |
The bold issue could easily be fixed by changing some compare from `this.currentOption === section.title` to `this.currentOption.toLowerCase() === section.title.toLowerCase`, but it comes with more issues. /src/utils/string.js export function anchorize(str) {
return str
.trim()
.replace(NON_URL_CHARS_RE, '-')
.replace(INITIAL_DASH_RE, '')
.toLowerCase();
} [Update]: This nav issue seems to be a bug under mobile mode(Which use MobileDropdown.vue internally), irrelevant to this issue. After your confirm we can open a new SR for it. And I'll make a case-insensitive compare PR to fix the origin issue |
Ah, I think I see the new issue you found (which is likely more of an issue than this one). It seems that we're generating the URLs in the mobile dropdown by utilizing the current route URL instead of just directly using the URL provided in the `references` data from the JSON. By relying on the JSON data, we should be able to better control these URLs since they will always be generated in the same way by DocC as opposed to the browser URL where the case can be changed by the user. I think it would be preferable to avoid using the `anchorize` utility function unless absolutely necessary, because ultimately we want these paths and URLs to be driven by exactly what the compiler is providing. There may be cases where we need to use it to create an anchor/fragment value, but hopefully in most places where that is needed, these values will be provided already by DocC. As you mentioned, we can still do a case insensitive comparison, but it should be a pretty uncommon situation for end users to hit once we are relying on the JSON data for URLs more completely. |
Pull Request in #11 |
Attachment: Download
Additional Detail from JIRA
md5: eae56a93152d6a940296e3aca2014b78
Issue Description:
When navigating to a tutorial page with a non-lowercased version of the URL, the navigation dropdown that allows users to move between different tutorials in a collection fails to bold the current tutorial.
For example, when navigating to: http://localhost:8000/tutorials/slothcreator/CREATING-CUSTOM-SLOTHS in the attached documentation catalog.
The primary dropdown doesn’t bold the current tutorial (Creating Custom Sloths) because the URL is not fully lowercased.
The primary dropdown should still bold the current tutorial regardless of URL casing.
The text was updated successfully, but these errors were encountered: