Skip to content

fix(core): fix dynamicTitle: false not taking effect in the toolbar. #33284

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.

Sign up for GitHub

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

Open
ia319 wants to merge 5 commits into
base: next
Choose a base branch
from
+10 −7
Open

fix(core): fix dynamicTitle: false not taking effect in the toolbar. #33284

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
09a3fdb
fix(core): add showSelectedLabel prop to Select to control selected l…
ia319 Dec 5, 2025
a3bc089
fix(core): disable Select selected-label override in ToolbarMenuSelect
ia319 Dec 5, 2025
6e1cae0
fix lint error
ia319 Dec 5, 2025
a0befe0
refactor (ToolbarMenuSelect): remove manual title calculation, remove…
ia319 Dec 6, 2025
b6ac5b3
Merge branch 'next' into bug/33281-dynamic-title-select
ia319 Dec 6, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion code/core/src/components/components/Select/Select.tsx
View file
Open in desktop
Copy link
Member

@Sidnioulz Sidnioulz Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'll need new stories showing how labels behave on !multiSelect and on multiSelect Selects, when a selection is made, and the option is on or off (so, 4 stories with defaultOptions and a play function validating the HTML output).

Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,11 @@ export interface SelectProps
onSelect?: (option: Value) => void;
onDeselect?: (option: Value) => void;
onChange?: (selected: Value[]) => void;
/**
* Show the selected option's title in place of the children when an option is selected.
* Defaults to true.
*/
showSelectedLabel?: boolean;
}

function valueToId(parentId: string, { value }: InternalOption | ResetOption): string {
Expand Down Expand Up @@ -200,6 +205,7 @@ export const Select = forwardRef<HTMLButtonElement, SelectProps>(
onChange,
tooltip,
ariaLabel,
showSelectedLabel = true,
...props
},
ref
Expand Down Expand Up @@ -501,7 +507,7 @@ export const Select = forwardRef<HTMLButtonElement, SelectProps>(
{!multiSelect && (
<>
{icon}
{selectedOptions[0]?.title ?? children}
{(showSelectedLabel && selectedOptions[0]?.title) || children}
</>
)}

Expand Down
1 change: 1 addition & 0 deletions code/core/src/toolbar/components/ToolbarMenuSelect.tsx
View file
Open in desktop
Copy link
Member

@Sidnioulz Sidnioulz Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Bunch more things to tidy up:

  • In docs/_snippets/storybook-preview-configure-globaltypes.md, could you please replace the 4 occurences of // Change title based on selected value to // Change title based on selected value (recommended for consistency with the Storybook UI)
  • Remove getSelectedTitle's source code as it is no longer needed

Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ export const ToolbarMenuSelect: FC<ToolbarMenuSelectProps> = withKeyboardCycle(
onReset={() => updateGlobals({ [id]: '_reset' })}
onSelect={(selected) => updateGlobals({ [id]: selected })}
icon={icon && <Icons icon={icon} __suppressDeprecationWarning={true} />}
showSelectedLabel={false}
Copy link
Contributor

@mrginglymus mrginglymus Dec 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
showSelectedLabel={false}
showSelectedLabel={dynamicTitle}

otherwise it'll never update

>
{title}
</Select>
Expand Down
Loading