[DataGrid] Add parameter to provide labels to resize UI #2585
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
✅ All tests passed successfully Details on your Workflow / Core Tests page. |
Summary - Unit Tests Code CoverageSummary
CoverageMicrosoft.FluentUI.AspNetCore.Components - 61.2%
|
dvoituron
previously requested changes
Aug 26, 2024
| [Parameter] | ||
| public string ResizeLabel { get; set; } = "Column width (in pixels)"; | ||
|
|
||
| /// <summary> |
Collaborator
There was a problem hiding this comment.
To avoid these multiple parameters, and group all this information in a single object, how about using something like this :
public record ResizeAria
{
public string? GrowLabel { get; set; }
public string? ShrinkLabel { get; set; }
public string? ResetLabel { get; set; }
public string? SubmitLabel { get; set; }
public static ResizeAria Default => new ResizeAria();
}The usage will be:
<FluentDataGrid ResizeAria="@(ResizeAria.Default with { GrowLabel = "...", ShrinkLabel = "..." } )">Do you find it too complex ?
Collaborator
Author
There was a problem hiding this comment.
I was thinking about something like that as well (but then with a dictionary of some sorts)
Collaborator
Author
There was a problem hiding this comment.
A record has been added. See updates in PR message above
Collaborator
There was a problem hiding this comment.
-
I find the syntax for defining the record clearer using property declarations (as above). And using your syntax, we don't have XML comments / property documentation.
-
Why did you put this class in the
Infrastructurefolder and not in theDataGrid/Columnsfolder? -
I think we need a static property
Defaultto create an instance of this object. Thus, the developer can initialize the parameter usingResizeAria.Default with { GrowLabel = “ ...”, ShrinkLabel = “ ...” } -
⚠️ You can't force RPs to merge: without explicit approval, we can continue discussing.
Collaborator
Author
There was a problem hiding this comment.
- By doing it this way, the developer does not have to do anything to use the default values. I think we can put the comments about the fields of the record in the parameter declaration in the Grid
- Yes, that is a mistake. Will move them
I could not get your variant to work with implemented defaults. I don't want to force the developer to have to supply values in the default caseI figured it out. Will change it in the other PR- I know but I wanted this in because (and was too impatient). I need it in the other grid thing I'm working on (PR inbound) (mea culpa). We can still discuss and change because we are the end bosses.
…dand can now be set throught the record as well
vnbaaij
changed the title
dannyldj
pushed a commit
to dannyldj/fluentui-blazor
that referenced
this pull request
Sep 26, 2024
* Add resize buttons aria labels * Use a record to group resize aria labels. ResizeLabel has been removedand can now be set throught the record as well * Adjust example to new 2024 dataset
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Fix #2507 by adding a new
ColumnResizeLabelsrecord which contains the following properties:DiscreteLabelExactLabelGrowAriaLabelShrinkAriaLabelResetAriaLabelSubmitAriaLabelAll properties are assigned default values. A parameter of this type with name
ColumnResizeLabelsis added to DataGid. This can be used to supply custom strings for the mentioned properties which will then show up in the resize options UINOTE the DataGrid parameter
ResizeLabelhas been superseded by this new parameter. If you were using a custom text for this, then change this to use theDiscreteLabeland/orExactLabel(depending on theResizeTypeyou are using)