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

Tutorial navigation selected state #1904

Merged
merged 3 commits into from
Mar 7, 2025

Conversation

tcjr
Copy link
Contributor

@tcjr tcjr commented Mar 7, 2025

Fixes #1877

The Selection component shows the select/optgroup/options for all the tutorials. It gets the data to render the options from the grouped property of the DocsService and calls the transitionTo method on the RouterService when a tutorial is selected.

When rendering each option, the selected attribute was not taking into account the group, so it was comparing the tutorial path part with the full current path and they would never match.

This PR updates the isSelected helper so it accepts both the group and tutorial.

I also added some component tests that use mocks for the two injected services.

Copy link

stackblitz bot commented Mar 7, 2025

Review PR in StackBlitz Codeflow Run & review this pull request in StackBlitz Codeflow.

@tcjr
Copy link
Contributor Author

tcjr commented Mar 7, 2025

I couldn't figure out why assert.dom() was failing the TS lint.

Property 'dom' does not exist on type 'Assert'

@NullVoxPopuli
Copy link
Owner

do you have an example of how I can see the failure case in action?
I'm looking on the currently deployed site, and I'm not sure.

thank you for fixing this tho!
I just want to make sure I fully understand the problem

@tcjr
Copy link
Contributor Author

tcjr commented Mar 7, 2025

The linked issue has a great video of the issue.

Essentially: select any tutorial using the selector. Then, open the selector again. Notice the checked tutorial did not update.

@NullVoxPopuli
Copy link
Owner

ah! my issue is that I have no checkmark on firefox 🙈 lol

alrighty -- I'll merge once green. thank you!

@NullVoxPopuli NullVoxPopuli merged commit 006776a into NullVoxPopuli:main Mar 7, 2025
8 checks passed
@github-actions github-actions bot mentioned this pull request Mar 7, 2025
@NullVoxPopuli NullVoxPopuli added bug Something isn't working documentation Improvements or additions to documentation labels Mar 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Tutorial navigation selected state does not update correctly
2 participants