feat: scroll to specific setting when opening settings dialog#8761
Merged
feat: scroll to specific setting when opening settings dialog#8761
Conversation
dosubot
bot
added
the
size:L
|
Playwright: ❌ 523 passed, 1 failed · 1 flaky ❌ Failed Tests📊 Browser Reports
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/10/2026, 09:52:27 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds "do not ask again" checkboxes and a "Re-enable in Settings" link in multiple dialogs; extends dialog service to accept a settingId, threads scrollToSettingId into settings UI, and implements scrolling + highlight to a target setting item. Changes
Sequence DiagramsequenceDiagram
participant User
participant DialogContent as Dialog Content
participant DialogService as Dialog Service
participant SettingsDialog as Settings Dialog
participant UseSettingUI as useSettingUI
participant SettingStore as Setting Store
participant DOM as DOM/Renderer
User->>DialogContent: Click "Re-enable in Settings"
DialogContent->>DialogService: showSettingsDialog(panel?, settingId)
DialogService->>SettingsDialog: open(props: { defaultPanel, scrollToSettingId })
SettingsDialog->>UseSettingUI: useSettingUI(defaultPanel, scrollToSettingId)
UseSettingUI->>SettingStore: getSettingInfo(scrollToSettingId)
SettingStore-->>UseSettingUI: setting info (category)
UseSettingUI-->>SettingsDialog: resolved defaultCategory
SettingsDialog->>DOM: nextTick -> find [data-setting-id="..."]
SettingsDialog->>DOM: scrollIntoView() + add highlight (rgba(255, 223, 93, 0.5))
DOM-->>SettingsDialog: animationend
SettingsDialog->>DOM: remove highlight
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
No actionable comments were generated in the recent review. 🎉 🧹 Recent nitpick comments
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.2 kB (baseline 22.2 kB) • 🔴 +1 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 855 kB (baseline 855 kB) • 🔴 +1 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.8 kB (baseline 68.8 kB) • 🔴 +10 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed Panels & Settings — 451 kB (baseline 451 kB) • 🔴 +9 BConfiguration panels, inspectors, and settings screens
Status: 11 added / 11 removed User & Accounts — 16 kB (baseline 16 kB) • 🔴 +4 BAuthentication, profile, and account management bundles
Status: 7 added / 7 removed Editors & Dialogs — 751 B (baseline 751 B) • ⚪ 0 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 36.6 kB (baseline 36.6 kB) • 🔴 +8 BReusable component library chunks
Status: 10 added / 10 removed Data & Services — 2.12 MB (baseline 2.12 MB) • 🔴 +1.03 kBStores, services, APIs, and repositories
Status: 14 added / 14 removed Utilities & Hooks — 237 kB (baseline 237 kB) • 🔴 +1 BHelpers, composables, and utility bundles
Status: 15 added / 15 removed Vendor & Third-Party — 8.77 MB (baseline 8.77 MB) • 🔴 +16 BExternal libraries and shared vendor chunks
Status: 6 added / 6 removed Other — 7.21 MB (baseline 7.21 MB) • 🔴 +4.26 kBBundles that do not match a named category
Status: 88 added / 88 removed |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In `@src/components/dialog/content/ConfirmationDialogContent.vue`:
- Around line 34-46: Replace the raw <button> used for the "re-enable" link in
ConfirmationDialogContent.vue (inside the doNotAskAgain block) with the shared
TextButton (or IconTextButton if an icon is needed) so theming and focus styles
are consistent: import TextButton from the button components, add it to the
component's components map, and replace the element with <TextButton
class="underline cursor-pointer p-0 text-sm text-muted-foreground"
`@click`="openBlueprintOverwriteSetting">{{
t('missingModelsDialog.reEnableInSettingsLink') }}</TextButton> (preserve the
i18n-t template/link slot logic and the `@click` handler on
openBlueprintOverwriteSetting). Ensure you remove the raw button-specific
attributes (bg-transparent, border-none) since the TextButton handles styling.
In `@src/components/dialog/content/MissingModelsWarning.vue`:
- Around line 21-27: Replace the raw <button> in MissingModelsWarning.vue's
template slot "#link" with the repo's shared TextButton (or IconTextButton)
component to preserve design-system styles and focus behavior: import TextButton
from the button components directory at the top of the component, register it in
the components section, then swap the <button ...
`@click`="openShowMissingModelsSetting">...</button> for <TextButton
`@click`="openShowMissingModelsSetting">{{
t('missingModelsDialog.reEnableInSettingsLink') }}</TextButton> (remove the
manual classes applied to the raw button and rely on TextButton's styling);
ensure the click handler name openShowMissingModelsSetting remains unchanged so
behavior is preserved.
In `@src/components/dialog/content/MissingNodesFooter.vue`:
- Around line 5-9: The PrimeVue <Checkbox> usage (v-model="doNotAskAgain",
input-id="doNotAskAgainNodes") should be replaced with a native input element
bound to the same model: add an <input id="doNotAskAgainNodes"
v-model="doNotAskAgain" type="checkbox"> and apply Tailwind classes (e.g., h-4
w-4 cursor-pointer) to match the project's style pattern (see
ImageLayerSettingsPanel.vue for reference), and remove the corresponding
PrimeVue Checkbox import (the imported symbol "Checkbox") from the component
imports.
🧹 Nitpick comments (1)
src/platform/settings/components/SettingDialogContent.vue (1)
206-232: One-shot scroll attempt may silently miss if the panel DOM isn't ready bynextTick.The
settledflag makes this a single-attempt scroll. If the target element hasn't rendered by the firstnextTick(e.g., due to lazy tab rendering, async content, or transitions),elwill benulland the scroll/highlight silently skips with no retry.Since
useSettingUIalready resolves the correct category up front and the tab is set immediately, this should work for the common case. But consider usingrequestAnimationFrameor a shortsetTimeoutas a more resilient alternative tonextTickfor DOM-dependent operations after tab transitions:Suggested resilience improvement
void nextTick(() => { stopScrollWatch() - const el = document.querySelector( - `[data-setting-id="${CSS.escape(scrollToSettingId)}"]` - ) - if (!el) return - el.scrollIntoView({ behavior: 'smooth', block: 'center' }) - el.classList.add('setting-highlight') - el.addEventListener( - 'animationend', - () => el.classList.remove('setting-highlight'), - { once: true } - ) + requestAnimationFrame(() => { + const el = document.querySelector( + `[data-setting-id="${CSS.escape(scrollToSettingId)}"]` + ) + if (!el) return + el.scrollIntoView({ behavior: 'smooth', block: 'center' }) + el.classList.add('setting-highlight') + el.addEventListener( + 'animationend', + () => el.classList.remove('setting-highlight'), + { once: true } + ) + }) })
Myestery
force-pushed
the
feat/scroll-to-setting
branch
from
0aebf60 to
6f679a8
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@src/components/dialog/content/ConfirmationDialogContent.vue`:
- Around line 30-32: The template contains a native <label> element in
ConfirmationDialogContent.vue with an invalid PrimeVue prop
severity="secondary"; remove the severity="secondary" attribute from the label
(the element rendering the doNotAskAgain text) so the markup is valid and relies
on CSS/classes for styling instead.
In `@src/platform/settings/components/SettingDialogContent.vue`:
- Around line 206-232: The watcher around tabValue currently sets settled=true
immediately and calls stopScrollWatch() before confirming the target element
exists, causing retries to be skipped; modify the logic in the scroll-to block
that uses scrollToSettingId, settled, stopScrollWatch, watch(tabValue,...),
nextTick and onBeforeUnmount so that settled and stopScrollWatch() are only
set/called after
document.querySelector(`[data-setting-id="${CSS.escape(scrollToSettingId)}"]`)
successfully returns an element; keep the smooth scroll, add/remove of
'setting-highlight', and the animationend listener the same, but if the element
is not found, do not mark settled or stop the watcher so it can retry on
subsequent tabValue changes (and still clean up with onBeforeUnmount).
🧹 Nitpick comments (2)
src/components/dialog/content/MissingNodesFooter.vue (1)
96-98: Minor inconsistency: watcher-based persistence vs.onBeforeUnmount.
MissingModelsWarning.vuepersistsdoNotAskAgaininonBeforeUnmount, while this component uses awatchthat writes to the store on every toggle. Both work, but the watcher approach means an unnecessary store write when the user unchecks. Consider aligning with theonBeforeUnmountpattern for consistency, or keeping this if the intent is to persist immediately.src/platform/settings/composables/useSettingUI.test.ts (1)
45-51: Consider importingSettingParamsinstead of defining a local mock interface.
MockSettingParamsduplicates the shape ofSettingParams. Importing the real type keeps the mock aligned and avoids drift ifSettingParamschanges.Suggested change
-interface MockSettingParams { - id: string - name: string - type: string - defaultValue: unknown - category?: string[] -} +import type { SettingParams } from '@/platform/settings/types'Then use
Partial<SettingParams>orPick<SettingParams, 'id' | 'name' | 'type' | 'defaultValue'>for the mock record type.Based on learnings: "In TypeScript test files, avoid duplicating interface/type definitions. Import real type definitions from the component modules under test."
Add settingId parameter to showSettingsDialog that auto-navigates to the correct category tab, scrolls to the setting, and briefly highlights it. Add "don't show again" checkboxes with "re-enable in Settings" links to missing nodes, missing models, and blueprint overwrite dialogs.
Myestery
force-pushed
the
feat/scroll-to-setting
branch
from
6f679a8 to
699693d
Compare
| if (settled) return | ||
| settled = true | ||
| void nextTick(() => { | ||
| stopScrollWatch() |
Member
There was a problem hiding this comment.
stopScrollWatch() is called before verifying the target element exists. With immediate: true, this can run before lazy tab content is in the DOM, so querySelector may return null and we never retry.
| type="checkbox" | ||
| class="h-4 w-4 cursor-pointer" | ||
| /> | ||
| <label for="doNotAskAgain" severity="secondary">{{ |
Member
There was a problem hiding this comment.
nit: severity="secondary" is a PrimeVue-style prop, but this is a native <label>.
| @@ -0,0 +1,139 @@ | |||
| import { createPinia, setActivePinia } from 'pinia' | |||
benceruleanlu
previously approved these changes
Feb 10, 2026
- Remove invalid severity="secondary" from native <label> element - Fix scroll watcher premature settlement by only stopping after element is found
benceruleanlu
approved these changes
Feb 10, 2026
viva-jinyi
added a commit
that referenced
this pull request
Feb 15, 2026
## Summary Restores the scroll-to-setting and highlight animation that was lost during the BaseModalLayout migration in #8270. Originally implemented in #8761. ## Changes - **What**: Re-added scroll-into-view + pulse highlight logic and CSS animation to `SettingDialog.vue`, ported from the deleted `SettingDialogContent.vue` Fixes #3437 ┆Issue is synchronized with this [Notion page](https://www.notion.so/PR-8833-bugfix-Restore-scroll-to-setting-in-SettingDialog-3056d73d36508161abeee047a40dc1e5) by [Unito](https://www.unito.io)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds
settingIdparameter toshowSettingsDialogthat auto-navigates to the correct category tab, scrolls to the setting, and briefly highlights it with a CSS pulse animationAdds
data-setting-idattributes to setting items for stable DOM targetingAdds "Don't show this again" checkbox with "Re-enable in Settings" deep-link to the missing nodes dialog
Adds "Re-enable in Settings" deep-link to missing models and blueprint overwrite "Don't show this again" checkboxes
Fixes [Feature Request]: Add arg to "go to setting" when opening settings dialog #3437
Test plan
pnpm typecheckpassespnpm lintpassesuseSettingUI)Screen.Recording.2026-02-10.at.01.47.41.mov
┆Issue is synchronized with this Notion page by Unito