Skip to content

MudAutocomplete, MudSelect: Allow popover customization and add scroll events#10327

Merged
ScarletKuro merged 12 commits intoMudBlazor:devfrom
versile2:fix-popoverproperties
Nov 29, 2024
Merged

MudAutocomplete, MudSelect: Allow popover customization and add scroll events#10327
ScarletKuro merged 12 commits intoMudBlazor:devfrom
versile2:fix-popoverproperties

Conversation

@versile2
Copy link
Contributor

@versile2 versile2 commented Nov 25, 2024

Description

Allow MudSelect and MudAutocomplete to pass Fixed and OverflowBehavior to the contained popover. Defaults are the same as before, just now it can be overridden if need be.

Additionally, add scroll events on connect for parents with scrollable containers

Resolves #8656
Resolves #10325
Resolves doc page where there is a floating component containing a popover by setting the AutoComplete to Fixed="true"

How Has This Been Tested?

Visual Tests

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.

@github-actions github-actions bot added bug Unexpected behavior or functionality not working as intended docs Updates/improvements to project documentation that do not affect core library logic enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library PR: needs review labels Nov 25, 2024
@versile2
Copy link
Contributor Author

My gif creator did not work well this time but see below

Problem
badbehaviorproperties
Fix
propertiespassedfix

@versile2
Copy link
Contributor Author

Example for 8656, There's another one I fixed but I lost the # while working.

scrollablefix

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.

Just some nits.

@codecov
Copy link

codecov bot commented Nov 25, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.48%. Comparing base (ac1d69c) to head (627ffa5).
Report is 10 commits behind head on dev.

Additional details and impacted files
@@            Coverage Diff             @@
##              dev   #10327      +/-   ##
==========================================
- Coverage   91.54%   91.48%   -0.07%     
==========================================
  Files         415      416       +1     
  Lines       13017    13054      +37     
  Branches     2457     2473      +16     
==========================================
+ Hits        11917    11943      +26     
+ Misses        549      548       -1     
- Partials      551      563      +12     

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

@henon henon changed the title Pass Properties to Popover MudPopover: Allow overriding popover properties and add scroll events Nov 28, 2024
@henon henon changed the title MudPopover: Allow overriding popover properties and add scroll events MudPopover: Allow overriding properties and add scroll events Nov 28, 2024
@henon
Copy link
Contributor

henon commented Nov 28, 2024

What about merging these into a single parameter public DropDownBehavior DropdownBehavior { get; set; } with

public struct DropdownBehavior {
  public bool Fixed { get; set; }
  public OverflowBehavior OverflowBehavior { get; set; } = OverflowBehavior.FlipOnOpen;
}

Alternative names DropdownSettings, PopoverBehavior or PopoverSettings.

I think for this very specialized configuration preparing the settings object in@code and passing it like such <MudAutocomplete DropdownSettings="@settings"> is no big deal.

In addition to that the parameters intended for the popover are now intuitively named which is not the case when they are directly exposed in MudAutocomplete on their own. I when first seeing this I wasn't sure what <MudAutocomplete Fixed="true"> would mean.

@henon henon changed the title MudPopover: Allow overriding properties and add scroll events MudAutocomplete, MudSelect: Allow overriding popover properties and add scroll events Nov 28, 2024
@henon henon removed docs Updates/improvements to project documentation that do not affect core library logic PR: needs review labels Nov 28, 2024
@henon henon changed the title MudAutocomplete, MudSelect: Allow overriding popover properties and add scroll events MudAutocomplete, MudSelect: Allow popover customization and add scroll events Nov 28, 2024
@henon
Copy link
Contributor

henon commented Nov 28, 2024

I think for MudSelect there is potential for a future breaking change (i.e. v9) to move these placement properties into the new parameter as well.

image

Maybe then we can expand this example with the new options as well

@henon
Copy link
Contributor

henon commented Nov 28, 2024

Please name it DropDownSettings so we are not limited to behavioral settings.

@henon
Copy link
Contributor

henon commented Nov 28, 2024

Sorry for being a PITA, just think DropdownSettings would be best (no capitalization of Down)

@versile2
Copy link
Contributor Author

Sorry for being a PITA, just think DropdownSettings would be best (no capitalization of Down)

Well I'll be a monkey's uncle! Give me 5.

@versile2
Copy link
Contributor Author

and maybe 5 more, have to write unit tests

@versile2
Copy link
Contributor Author

Let me know if you want any additional changes, I hear PopoverSettings is hot right now :)

Copy link
Contributor

@henon henon left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for your patience ;). @ScarletKuro you may merge this if you agree with the changes.

@ScarletKuro
Copy link
Member

ScarletKuro commented Nov 28, 2024

LGTM, thanks for your patience ;). @ScarletKuro you may merge this if you agree with the changes.

The original problem that I initially reported on discord is fixed, but I see that in Edge if you open a very high inspector, this part is sticking out:
sticking out
Is it something that is possible to fix? Should we just merge this PR and leave this problem for other PR?

@versile2
Copy link
Contributor Author

LGTM, thanks for your patience ;). @ScarletKuro you may merge this if you agree with the changes.

The original problem that I initially reported on discord is fixed, but I see that in Edge if you open a very high inspector, this part is sticking out: sticking out Is it something that is possible to fix? Should we just merge this PR and leave this problem for other PR?

I'm sure it is, probably related to appbar.razor which I had to change for the new properties so I don't see a problem with adding it to this PR. give me a few to confirm and I'll submit if I'm correct.

@versile2
Copy link
Contributor Author

Actually no, it all appears to be working by design. I can only see the overlap if I zoom to 200% and browser zoom can be odd anyways. I can replicate on chrome if I zoom high enough (500%), Same with firefox (220%). Unless there is some other way to replicate.

@ScarletKuro
Copy link
Member

Actually no, it all appears to be working by design. I can only see the overlap if I zoom to 200% and browser zoom can be odd anyways. I can replicate on chrome if I zoom high enough (500%), Same with firefox (220%). Unless there is some other way to replicate.

Hmm weird, I don't need to zoom anything. I have 2k monitor and all I have to do is to open inspector on like half of the screen size for it to stick out.
@danielchalmers should we bother about it?

@ScarletKuro ScarletKuro self-assigned this Nov 29, 2024
@danielchalmers
Copy link
Member

@ScarletKuro it's an old issue #8657

@ScarletKuro
Copy link
Member

@ScarletKuro it's an old issue #8657

Ok, lets merge then.

@ScarletKuro ScarletKuro merged commit ef4b998 into MudBlazor:dev Nov 29, 2024
@versile2 versile2 deleted the fix-popoverproperties branch November 30, 2024 20:15
LLauter pushed a commit to cannellamedia/MudBlazor that referenced this pull request Dec 16, 2024
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 enhancement Adds a new feature or enhances existing functionality (not fixing a defect) in the main library

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Autocomplete Popover inside DataGrid extends page instead of flipping MudSelect popover content position not relative to MudDrawer

4 participants