Skip to content

[select] add data-placeholder#2737

Merged
atomiks merged 19 commits intomui:masterfrom
seongminn:with-object-values
Nov 13, 2025
Merged

[select] add data-placeholder#2737
atomiks merged 19 commits intomui:masterfrom
seongminn:with-object-values

Conversation

@seongminn
Copy link
Contributor

@seongminn seongminn commented Sep 11, 2025 •
edited
Loading

based on #2704

  • Add the data-placeholder attribute to Select.Value when a placeholder item with no value is selected. This allows for applying custom styles. (1d928aa)
  • I've opened the PR to address this feedback.

@mui-bot
Copy link

mui-bot commented Sep 11, 2025 •
edited
Loading

Bundle size report

Bundle Parsed size Gzip size
@base-ui-components/react 🔺+213B(+0.05%) 🔺+145B(+0.12%)

Details of bundle changes

Check out the code infra dashboard for more information about this PR.

@netlify
Copy link

netlify bot commented Sep 11, 2025 •
edited
Loading

✅ Deploy Preview for base-ui ready!

Name Link
🔨 Latest commit b58730e
🔍 Latest deploy log https://app.netlify.com/projects/base-ui/deploys/69154780664e7a00082a541d
😎 Deploy Preview https://deploy-preview-2737--base-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@seongminn seongminn mentioned this pull request Sep 11, 2025
Merged
1 task
@zannager zannager added the docs Improvements or additions to the documentation. label Sep 12, 2025
@seongminn seongminn changed the title With object values [select] add data-placeholder Sep 12, 2025
@zannager zannager added the component: select Changes related to the select component. label Sep 15, 2025
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Sep 18, 2025
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Oct 2, 2025
@pkg-pr-new
Copy link

pkg-pr-new bot commented Oct 2, 2025 •
edited
Loading

vite-css-base-ui-example

pnpm add https://pkg.pr.new/mui/base-ui/@base-ui-components/react@2737

pnpm add https://pkg.pr.new/mui/base-ui/@base-ui-components/utils@2737

commit: b58730e

@seongminn
Copy link
Contributor Author

seongminn commented Oct 2, 2025

@atomiks The base PR for this pull request has been merged into master. Could you please review this PR?

@atomiks
Copy link
Contributor

atomiks commented Nov 5, 2025

@seongminn super sorry, this PR got away from me and I forgot to review it!

Some thoughts:

  • Should Select.Trigger also have this state?
  • Can you add tests for this state attribute (direct null, object with value: null, custom property with itemToStringValue)

@atomiks atomiks added type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature. and removed docs Improvements or additions to the documentation. labels Nov 5, 2025
@seongminn
Copy link
Contributor Author

seongminn commented Nov 10, 2025

@atomiks
Sorry for the late reply 😅
All requirements have been incorporated.

I always appreciate your feedback.

atomiks
atomiks reviewed Nov 11, 2025
View reviewed changes
packages/react/src/select/trigger/SelectTrigger.tsx Outdated
Comment on lines +96 to +101
const serializedValue = React.useMemo(() => {
if (isMultiple && Array.isArray(value) && value.length === 0) {
return '';
}
return stringifyAsValue(value, itemToStringValue);
}, [isMultiple, value, itemToStringValue]);
Copy link
Contributor

@atomiks atomiks Nov 11, 2025

Choose a reason for hiding this comment

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

I think this also exists in the root, the logic could be reused between the three sites

Copy link
Contributor Author

@seongminn seongminn Nov 11, 2025

Choose a reason for hiding this comment

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

I have included serializedValue in the SelectRootContext! 38c7763

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged. label Nov 11, 2025
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged. label Nov 11, 2025
atomiks
atomiks reviewed Nov 12, 2025
View reviewed changes
packages/react/src/select/root/SelectRoot.tsx Outdated
Comment on lines +408 to +416
@@ -406,6 +413,7 @@ export function SelectRoot<Value, Multiple extends boolean | undefined = false>(
disabled,
readOnly,
multiple,
serializedValue,
Copy link
Contributor

@atomiks atomiks Nov 12, 2025

Choose a reason for hiding this comment

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

This should be placed in the store sync effect where store.update(...) is called (like value is).
Also it seems serializedValue: createSelector((state: State) => { should be removed.

Copy link
Contributor Author

@seongminn seongminn Nov 12, 2025

Choose a reason for hiding this comment

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

I have incorporated all the points you mentioned!

@atomiks
Copy link
Contributor

atomiks commented Nov 13, 2025

Thanks for the contribution!

@atomiks atomiks merged commit 97f9d3f into mui:master Nov 13, 2025
20 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@atomiks atomiks atomiks approved these changes

@colmtuite colmtuite Awaiting requested review from colmtuite colmtuite is a code owner

@michaldudak michaldudak Awaiting requested review from michaldudak michaldudak is a code owner

Assignees

No one assigned

Labels

component: select Changes related to the select component. type: enhancement It’s an improvement, but we can’t make up our mind whether it's a bug fix or a new feature.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@seongminn @mui-bot @atomiks @zannager