Fix. Quickgrid PropertyColumn Align property doesn't work with Align.Right #62707
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request fixes a bug where the QuickGrid PropertyColumn's Align property doesn't work correctly with right alignment (Align.Right). The fix refactors CSS styles to properly support column alignment and adds missing styles for consistent column options positioning.
- Reorganizes existing CSS rules for better logical grouping
- Adds support for
col-justify-startstyles for left alignment consistency - Introduces explicit
col-justify-rightandcol-justify-leftstyles for direct alignment control - Adds RTL (right-to-left) support for the new
col-justify-startstyles
| td.col-justify-right { | ||
| text-align: right; | ||
| } | ||
|
|
||
| td.col-justify-left { | ||
| text-align: left; | ||
| } |
There was a problem hiding this comment.
The styles td.col-justify-right and td.col-justify-left appear to duplicate the functionality of existing td.col-justify-end and td.col-justify-start styles. This creates redundant CSS rules that could lead to maintenance issues. Consider using a single consistent naming convention.
| td.col-justify-right { | |
| text-align: right; | |
| } | |
| td.col-justify-left { | |
| text-align: left; | |
| } |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Can't we use these existing styles as a fix, instead of adding new ones?
There was a problem hiding this comment.
I thought the additional classes were so that they can be overwritten in different scenarios (in particular when the page is set to Right-To-Left text direction). But maybe I misunderstood it, CSS is harder to mentally parse than lambda calculus 😄
| @@ -76,10 +71,32 @@ td.col-justify-center { | |||
| text-align: center; | |||
| } | |||
|
|
|||
| .col-justify-end .col-options { | |||
There was a problem hiding this comment.
Let's move it back to the original position. Moving it few lines does not change the functionality but might be confusing later to read the change history, e.g. when using git blame
| td.col-justify-right { | ||
| text-align: right; | ||
| } | ||
|
|
||
| td.col-justify-left { | ||
| text-align: left; | ||
| } |
There was a problem hiding this comment.
Can't we use these existing styles as a fix, instead of adding new ones?
Fix. Quickgrid PropertyColumn Align property doesn't work with Align.Right
Description
This pull request refactors and fixes the column alignment and justification styles in the
QuickGrid.razor.cssfile.Changes:
.col-justify-start .col-optionsandtd.col-justify-startto support left-aligned column options and text.td.col-justify-rightandtd.col-justify-leftstyles for explicit right and left text alignment.Reasoning for change:
In the
aspnetcore\src\Components\QuickGrid\Microsoft.AspNetCore.Components.QuickGrid\src\QuickGrid.razor.csBlazor assigns css classes based on the Align parameter that user has set. However, in thesrc\Components\QuickGrid\Microsoft.AspNetCore.Components.QuickGrid\src\QuickGrid.razor.cssthere were no implementation for 3 out of existing 5 parameter options. This PR fixes implements missing options.Fixes #50029