Feature/improve settings navigation#617
Feature/improve settings navigation#617Elodie-DeMatteis-Upf wants to merge 13 commits intomasterfrom
Conversation
Elodie-DeMatteis-Upf
force-pushed
the
feature/improve-settings-navigation
branch
from
4507a48 to
e77b342
Compare
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| {{/if}} | ||
|
|
||
| <div class="oss-sidebar-item__label"> | ||
| <div class={{concat "oss-sidebar-item__label" (unless @icon " oss-sidebar-item__label--no-icon")}}> |
There was a problem hiding this comment.
Label no-icon class ignores icon named block usage
Medium Severity
The oss-sidebar-item__label--no-icon class is applied based solely on @icon being falsy via (unless @icon ...), but this doesn't account for when an icon is rendered via the named block. When using <:icon> without providing @icon (a valid pattern used in sidebar.hbs for the home logo), the label incorrectly receives the --no-icon class with extra left padding, even though an icon is visually present.
upfluence-bot
added
reviewed/needs-work
reviewed/needs-review
and removed
reviewed/needs-review
reviewed/needs-work
labels
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
Bugbot Autofix is OFF. To automatically fix reported issues with Cloud Agents, enable Autofix in the Cursor dashboard.
| const route = this.args.routePrefix ? this.args.routePrefix + '.' + this.args.link : this.args.link; | ||
| try { | ||
| return Boolean(this.router.urlFor(this.args.link)); | ||
| return Boolean(this.router.urlFor(route)); |
There was a problem hiding this comment.
LinkTo route mismatch between validation and navigation
High Severity
The isInternalRoute getter validates whether routePrefix + '.' + link exists as a route, but the template passes only @link (without the prefix) to <LinkTo @route={{@link}}>. When routePrefix="display" and link="index", the component validates that "display.index" exists, but then creates a link that navigates to "index" instead. This causes navigation to the wrong route or a route error.
Additional Locations (1)
| export const BasicUsage = DefaultUsageTemplate.bind({}); | ||
| BasicUsage.args = { | ||
| link: 'https://www.upfluence.com', | ||
| disableAutoActive: false, |
There was a problem hiding this comment.
Storybook args defined but not used in template
Low Severity
The disableAutoActive property is defined in argTypes and added to BasicUsage.args, but the story template at line 71 does not pass @disableAutoActive={{this.disableAutoActive}} or @routePrefix={{this.routePrefix}} to the component. This creates non-functional Storybook controls that appear in the UI but have no effect when changed, which is confusing for developers using Storybook to explore the component's API.
Additional Locations (1)
Elodie-DeMatteis-Upf
force-pushed
the
feature/improve-settings-navigation
branch
from
7a85ce9 to
695be67
Compare
2 participants
What does this PR do?
Related to: #
What are the observable changes?
Good PR checklist
Note
Medium Risk
Touches routing/active-link behavior and sidebar expansion state (including localStorage-driven UI), which could subtly change navigation highlighting or layout in consuming apps.
Overview
Updates
OSS::Anchorto support engine-style routing via@routePrefixand to optionally disable Ember’s automaticLinkToactive class via@disableAutoActive(wired through to@current-when).Enhances
OSS::Layout::Sidebarwith an optional:headernamed block and an@alwaysExpandedmode that forces the expanded state and hides the expand/collapse toggle. Sidebar items now pass through the new anchor routing args, avoid rendering an empty icon wrapper when@iconis missing, and add styling for icon-less labels; Storybook args and integration tests were updated accordingly.Written by Cursor Bugbot for commit 695be67. This will update automatically on new commits. Configure here.