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

[SR-15313] [Swift-DocC-Render] Primary tutorial navigation dropdown fails to bold the current tutorial when browser URL is not lowercased #282

Open
mportiz08 opened this issue Oct 13, 2021 · 5 comments
Assignees
Labels

Comments

@mportiz08
Copy link
Contributor

@mportiz08 mportiz08 commented Oct 13, 2021

Previous ID SR-15313
Radar rdar://59818008
Original Reporter @mportiz08
Type Bug
Status In Progress
Resolution

Attachment: Download

Additional Detail from JIRA
Votes 0
Component/s Swift-DocC-Render
Labels Bug, StarterBug
Assignee @Kyle-Ye
Priority Medium

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.

@Kyle-Ye
Copy link

@Kyle-Ye Kyle-Ye commented Oct 16, 2021

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

  1. We simply make the compare in the vue file to ignore the case so that it will set the isActive properly

  2. We redirect all path into lower-case one, so anyone try to access http://localhost:8000/tutorials/slothcreator/CREATING-CUSTOM-SLOTHS or http://localhost:8000/tutorials/slothcreator/CREATING-custom-SLOTHS will be redirected into http://localhost:8000/tutorials/ slothcreator /creating-custom-sloths

@mportiz08
Copy link
Contributor Author

@mportiz08 mportiz08 commented Oct 18, 2021

@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.

@Kyle-Ye
Copy link

@Kyle-Ye Kyle-Ye commented Oct 26, 2021

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.
Like if I navigate to `http://localhost:8080/tutorials/slothcreator/creating-custom-slothS` now it can render the bold properly after applying the first fix, but click the navigation bar did nothing, because the URL change to something like this `http://localhost:8080/tutorials/slothcreator/creating-custom-slothS#Add-a-customization-view` which can't be routed.
I think we change the routing rule to redirect the url to a lower_case style is more acceptable and consist with whole design. (Because the "Creating Custom Sloths" was transformed into "creating_custom_sloths" internally) @mportiz08

/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

@mportiz08
Copy link
Contributor Author

@mportiz08 mportiz08 commented Oct 27, 2021

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.

@Kyle-Ye
Copy link

@Kyle-Ye Kyle-Ye commented Oct 27, 2021

Pull Request in #11

@swift-ci swift-ci transferred this issue from apple/swift-issues Apr 25, 2022
@shahmishal shahmishal transferred this issue from apple/swift May 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants