Skip to content
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.

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

Allow ShowColumnOptionsAsync method to accept null parameter #56991

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

spdebeer
Copy link

Allow ShowColumnOptionsAsync method to accept null parameter

Description

Allow the ColumnBase<TGridItem> parameter passed into the ShowColumnOptionsAsync method to be NULL

Fixes #54357

@spdebeer spdebeer requested review from a team as code owners July 25, 2024 14:09
@dotnet-issue-labeler dotnet-issue-labeler bot added the area-blazor Includes: Blazor, Razor Components label Jul 25, 2024
@dotnet-policy-service dotnet-policy-service bot added the community-contribution Indicates that the PR has been added by a community member label Jul 25, 2024
Copy link
Contributor

Thanks for your PR, @spdebeer. Someone from the team will get assigned to your PR shortly and we'll get it reviewed.

@spdebeer spdebeer marked this pull request as draft July 25, 2024 15:12
@spdebeer spdebeer marked this pull request as ready for review July 26, 2024 09:55
@spdebeer
Copy link
Author

I see that the tests are unhappy with the modification I made to the PublicAPI.Shipped.txt file, I am however not sure how to proceed without that modification, the build seems to fail without it, having the updated method signature in the Unshipped file alone seems to not be enough. Any guidance or recommendations will be appreciated.

Copy link
Contributor

Looks like this PR hasn't been active for some time and the codebase could have been changed in the meantime.
To make sure no conflicting changes have occurred, please rerun validation before merging. You can do this by leaving an /azp run comment here (requires commit rights), or by simply closing and reopening.

@dotnet-policy-service dotnet-policy-service bot added the pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun label Aug 5, 2024
@spdebeer
Copy link
Author

spdebeer commented Aug 6, 2024

hi @MackinnonBuck, could you please have a look for me, would be much appreciated

@@ -264,7 +264,7 @@ public Task SortByColumnAsync(ColumnBase<TGridItem> column, SortDirection direct
/// options UI that was previously displayed.
/// </summary>
/// <param name="column">The column whose options are to be displayed, if any are available.</param>
public Task ShowColumnOptionsAsync(ColumnBase<TGridItem> column)
public Task ShowColumnOptionsAsync(ColumnBase<TGridItem>? column)
{
_displayOptionsForColumn = column;
_checkColumnOptionsPosition = true; // Triggers a call to JS to position the options element, apply autofocus, and any other setup
Copy link

@matthetherington matthetherington Sep 16, 2024

Choose a reason for hiding this comment

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

If called with null, this is still going to set _checkColumnOptionsPosition to true, resulting in unnecessary interop.

I'd suggest just exposing CloseColumnOptions instead.

Edit: I've PR'd that here: #57904

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-blazor Includes: Blazor, Razor Components community-contribution Indicates that the PR has been added by a community member pending-ci-rerun When assigned to a PR indicates that the CI checks should be rerun
Projects
None yet
Development

Successfully merging this pull request may close these issues.

QuickGrid - Expose way to Hide Column Options
3 participants