docs: say the parent default is set in the builder, not on any dashboard - #11690
Conversation
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
|
Claude finished @mngr's task in 36s —— View job Review complete — no blocking issues (0 high, 0 medium, 1 low)The one-line rewording is accurate, consistent with the filter section, and reads better. LGTM. Details
What changed
AssessmentCorrectness of the fix. The original lead ("the one you last picked in the control itself on the dashboard") did read as if a viewer's pick on a published dashboard could set the default, which contradicts both the following sentence and the "Published dashboards only" bullet at line 192-193. The new phrasing removes the ambiguity by naming the builder up front. Consistency. It now mirrors the static-default sentence at line 41 ("configured by interacting with the filter in the dashboard builder — the value you select is saved on the widget and applied to every viewer when the dashboard loads"), which is the right precedent to follow — a reader who skimmed the filter section one screen up will recognize the shape. Unmentioned improvement worth calling out. The old sentence also compared the parent default to "a time granularity switcher's default", which was wrong: per the Default granularity section (line 94-96), the switcher does have a configurable default in its settings, so it isn't set by interaction at all. Dropping that half of the comparison fixes a second, unrelated inaccuracy. Good that the narrower comparison to the filter's static default is the only one left. Downstream paragraphs still read correctly. Line 152 ("Picking in the builder also applies that option's values to the children…") and line 154 (never picking / deleting the default option) follow on cleanly from the new lead — no orphaned "the same way…" reference left behind. Low — bolding a field that doesn't exist
Not applicableSecurity, performance, and test-coverage checks are moot for a prose-only change to a single |
|
Claude finished @mngr's task in 57s —— View job Review complete — 0 high, 1 medium, 0 lowThe reworded lead is accurate and consistent with the filter section, and the follow-up commit resolved the earlier bolding nit. One new issue appeared from master moving underneath this branch. Details
What changed since the last review
Medium — the new sentence contradicts a section that landed on master after this branch
This branch forked before #11689, which added a User attribute default subsection directly beneath Default option on master:
So on master there is a default-related field in the parent's settings. Once this merges, line 150's absolute claim —
— sits one paragraph above a subsection that tells the reader to go find exactly such a field. A reader skimming top-down hits the denial first and may skip the subsection entirely. The filter section one screen up already models the fix: it splits Static default from User attribute default and scopes its "configured by interacting with the filter" sentence to the static case only (line 41). Mirroring that is a one-word change: Or, if you'd rather point forward explicitly: Note this only bites after a merge/rebase onto current master — the branch as it stands is internally consistent, so it's a merge-order artifact rather than a defect in the diff itself. Worth fixing here rather than leaving for a third follow-up. Still correct from the previous pass
Not applicableSecurity, performance, and test-coverage checks are moot for a prose-only change to one |
Follow-up to #11685, applying a review comment that landed on it before it was merged. "the one you last picked in the control itself on the dashboard" is ambiguous about which dashboard. A viewer changing a parent control on a PUBLISHED dashboard does not change the saved default — only a pick in the builder does — and the lead sentence is the one people skim. Mirror the phrasing the filter section already uses ("configured by interacting with the filter in the dashboard builder — the value you select is saved on the widget"), which also collapses the two sentences into one and keeps the paragraph its original length.
Review note on this PR: bold is used throughout controls.mdx for real UI labels (**Children**, **Reset to default**, **Visibility**), so bolding **Default value** for a field that explicitly is NOT there works against the convention — a reader skimming the bold text could come away thinking it exists.
#11689 landed on master while this branch was open, adding a **User attribute default** subsection directly beneath **Default option** — so there IS now a default-related field in the parent control's settings, and a flat "there's no default field" sits one paragraph above a subsection telling readers to go find exactly such a field. Scope it to the static default, the same split the filter section one screen up already makes between **Static default** and **User attribute default**.
393a398 to
1f42189
Compare
|
Claude finished @mngr's task in 41s —— View job Review complete — 0 high, 0 medium, 0 lowThe new commit resolves the medium finding from the last pass; the page is now internally consistent against current Details
What changed since the last review
That is exactly the fix the previous pass asked for. Verified against the rest of the page
Not applicableSecurity, performance, and test-coverage checks are moot for a prose-only change to a single |
Applies the review comment on #11685 that I merged past — my mistake, so this lands as a follow-up rather than an edit to that PR.
The finding was right. #11685 opened the Default option paragraph with:
which is ambiguous about which dashboard. A viewer changing a parent control on a published dashboard does not change the saved default — only a pick in the builder does. The following sentence said so, but the lead sentence is the one people skim, and on its own it reads as "whatever was last selected wins."
This uses the reviewer's suggested wording, which mirrors the phrasing the filter section already uses one screen up ("configured by interacting with the filter in the dashboard builder — the value you select is saved on the widget") and collapses two sentences into one:
Product change: cubedevinc/cubejs-enterprise#14501 (CUB-4201), merged.