Skip to content

[MudSelect] Control popover width#10215

Merged
danielchalmers merged 6 commits intoMudBlazor:devfrom
Anu6is:MudSelect-Popover
Nov 9, 2024
Merged

[MudSelect] Control popover width#10215
danielchalmers merged 6 commits intoMudBlazor:devfrom
Anu6is:MudSelect-Popover

Conversation

@Anu6is
Copy link
Contributor

@Anu6is Anu6is commented Nov 8, 2024

Description

MudSelect hardcoded the RelativeWidth of the MudPopover to true. This ensures that the popover width matches the select field width. However, when items available for selection are longer than the width of the select field, they are cut off. This may not always be desirable.

This change

  • Utilizes the FullWidth parameter from MudBaseInput
    • Forwards that value to the MudPopover component
    • Defaults to false which is the opposite of the current behavior (current behavior is considered a bug)
  • Updates mudPopover.js to set either min or max width based on the value of RelativeWidth

While a non js fix would be preferred, the current js is what enforces the max-width restriction and without setting min-width, the popover may render smaller than the select field, instead of matching its size, when RelativeWidth is false and the item list options are short in length.

Resolves #10195 , #825

How Has This Been Tested?

Visually - unit test viewer
Added a unit test to confirm applied popover classes

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

- Use user supplied RelativeWidth instead of hard coded value
- set min or max width depending on RelativeWidth
@github-actions github-actions bot added bug Unexpected behavior or functionality not working as intended PR: needs review labels Nov 8, 2024
@Anu6is Anu6is changed the title [MudSelect] [MudSelect] Control popover width Nov 8, 2024
@danielchalmers
Copy link
Member

cc @versile2

@codecov
Copy link

codecov bot commented Nov 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.24%. Comparing base (89ac6e2) to head (66afecf).
Report is 435 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #10215      +/-   ##
==========================================
+ Coverage   91.22%   91.24%   +0.02%     
==========================================
  Files         411      412       +1     
  Lines       12506    12536      +30     
  Branches     2439     2446       +7     
==========================================
+ Hits        11408    11438      +30     
  Misses        555      555              
  Partials      543      543              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ScarletKuro
Copy link
Member

ScarletKuro commented Nov 8, 2024

Wouldn't false be a better default? (I'd say current behavior is a bug)

@Anu6is
Copy link
Contributor Author

Anu6is commented Nov 8, 2024

Sure, just figured I'd leave the current behaviour as is. But yes, I could change that

@versile2
Copy link
Contributor

versile2 commented Nov 8, 2024

ahh we were just discussing this, FullWidth (which defaults to False) is used on MudMenu to do this and defaults to false. I would suggest matching this behavior especially if the default is going to be false anyways. Also AutoComplete is affected by the same limitation/bug.

@Anu6is
Copy link
Contributor Author

Anu6is commented Nov 8, 2024

Just saw the conversation on #10204. Based on that, should I use FullWidth in the MudSelect instead of RelativeWidth and default said FullWidth to false? This would allow the popover to exceed its parent by default and if the user sets it to true it would match the select field size.

@versile2
Copy link
Contributor

versile2 commented Nov 8, 2024

I would say so and match the behavior to mudautocomplete, fullwidth is inherited from mudinputbase for those two so no need to even add a property.

@Anu6is
Copy link
Contributor Author

Anu6is commented Nov 8, 2024

Yeah, already done

Copy link
Member

@danielchalmers danielchalmers left a comment

Choose a reason for hiding this comment

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

Looks great. Do you think we should have a horizontal scrollbar if the full width isn't wide enough to fit the widest items? Separate PR maybe

@versile2
Copy link
Contributor

versile2 commented Nov 9, 2024

I thought about that and I don't. I can't think of a valid use case where the page shouldn't grow right of content exceeds

@ScarletKuro ScarletKuro self-requested a review November 9, 2024 14:12
Copy link
Member

@ScarletKuro ScarletKuro left a comment

Choose a reason for hiding this comment

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

LGTM, feel free to merge @henon / @danielchalmers

@sonarqubecloud
Copy link

sonarqubecloud bot commented Nov 9, 2024

@danielchalmers
Copy link
Member

Good work!

@ScarletKuro
Copy link
Member

regression #10235

@Anu6is Anu6is deleted the MudSelect-Popover branch November 27, 2024 13:30
LLauter pushed a commit to cannellamedia/MudBlazor that referenced this pull request Dec 16, 2024
Co-authored-by: Artyom M <artem.melnikov@live.com>
@danielchalmers danielchalmers added regression Previously worked and now doesn't and removed accidental break labels Jul 1, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Unexpected behavior or functionality not working as intended regression Previously worked and now doesn't

Projects

None yet

Development

Successfully merging this pull request may close these issues.

MudSelect may cutting of options in popover

4 participants