DataViews: Right-align integer and number fields#75917
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
integer and number fields
|
I pushed f5519e0 to fix the author field, whose type is Because the fix involves updating the view (explicitely set the alignment for the author field), I've had to reset the view for the fix to make effect. Sharing to keep in mind during testing beta, in case that comes up. The solution is just resetting the view for the change to be applied. |
|
Size Change: +394 B (+0.01%) Total Size: 6.84 MB
ℹ️ View Unchanged
|
| export const defaultLayouts = { | ||
| table: {}, | ||
| table: { | ||
| layout: { |
There was a problem hiding this comment.
I'm not sure about this.. Here is how it looks like without applying this:
Note that by default we align the table header (th) but not the field because of the custom render. I think it's not an uncommon use case and that would require other consumers apply the same fix everywhere.
I was wondering if it would be better to apply the default align only when a field has not declared a custom render function.
We could probably do that during field normalization (no new public API) and introducing a new prop, something like a meta object for possible future cases. For example we could have a meta: { hasCustomRender: true }.
What do you think?
There was a problem hiding this comment.
I find that approach too complex :)
I prefer optimizing for the normal situation (integer/number left-aligned) and edge cases (e.g., author) can adapt. Until now, we didn't have any numbers in our tables and that's the reason it wasn't an issue for use, but other consumers have reported this preference.
One thing I'm open to is to mark this as a breaking change to give it more visibility.
There was a problem hiding this comment.
I find that approach too complex :)
Maybe a bit, but not too much IMO. Maybe we can reconsider an approach like that if we find themselves in need for more similar info/flags. In general you have way more context around DataViews, so I trust your judgement.
but other consumers have reported this preference.
That's why I suggested this approach. Do you know if those were using custom render functions?
One thing I'm open to is to mark this as a breaking change to give it more visibility.
Maybe that would be better and we need to update the docs for this case somewhere, right? Because field authors could end up in this align mismatch in table layout and would start wondering why this is happening.
There was a problem hiding this comment.
I thought about including this in the docs but documenting every style we apply in DataViews is fragile (it can change) and unsustainable (takes a lot of work to keep it up to date for a minimal, if any, benefit).
There was a problem hiding this comment.
The author label and alignment does look off, but is that fixed here or does it need input?
There was a problem hiding this comment.
That fixes it, but you may need to reset your view for them to apply (if you have a previous saved view, that'd take preference).
|
Looking at this, it feels like the correct thing to do, right-align numbers by default. What are the main risks and concerns about making that change? It sounds like the surfaced Author field misalignment is addressed, but that would be one. A question pertains to whether the column heading is also right aligned when numbers in the columns are. On that, I can see both right and left alignment for the column heading, regardless of column content alignment, working:
I may lean towards right aligning headers, but if there are any concerns with that, solved by keeping those left aligned, I think that's fine too. |
|
Flaky tests detected in 4fe1e5a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/22488002555
|
|
I’d favour right-aligning column headers to maintain a coherent reading axis. I think a left-aligned header paired with right-aligned cell content risks creating too much visual disconnect. |
The risks are:
I've just come to realize we've shipped saved views in WordPress 6.9, so I don't think we can ask users to reset their view because the author column is misaligned. I'll think about ways forward and report back. |
Do we need a user option to choose alignments for elements inside a column? I may be misunderstanding, but that feels like too many options to me. IMO if a column is right aligned, it's because the data source defines it as right aligned, implicitly or otherwise, and that's just how it is. Which I think would be fine. |
|
@jasmussen The issue is that how we store the user options and why they overwrite other configs the user cannot set (e.g., alignment). I need to look into how that works before sharing options forward because I wasn't involved in that work. |
|
Prepared a potential fix at #75959 |
98a746b to
4fe1e5a
Compare
|
I'm coming back to this after a little detour. The changes for the view persistence at #75971 have landed and I've rebased this to work with them. I need to do some more testing, but it's looking ready. My recommended testing steps are:
|
jameskoster
left a comment
There was a problem hiding this comment.
Testing well for me :)
|
|
||
| export function getActiveViewOverridesForTab( activeView ) { | ||
| const base = { | ||
| ...defaultLayouts.table, |
There was a problem hiding this comment.
That's the only thing that concerns me slightly. Should we add to base the defaults based on the current view.type?
I know that now it doesn't matter because other layouts are empty, but maybe it would be a tiny bit better if we either added the defaults based on view.type (new param in the function) or extract the table layout value to a different const, which would be the default for table and would be used here as well to indicate that we have some baseDefaultLayout for any type.
Makes sense?
There was a problem hiding this comment.
It's tricky because there's a circular dependency: this function's output is the input of useView, and useView's output is the view (with the type). I think we'd need useView to understand the defaultLayouts concept, similar to what I explored at #75959 which didn't get support from Riad. It's an idea worth revisiting (but not urgent for beta).
ntsekouras
left a comment
There was a problem hiding this comment.
Visually this looks good and tests well, thanks! I left a small comment that we could address, but it's a small one so I'm approving now.
Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: harshbhonsle <harshbhonsle08@git.wordpress.org>
|
I just cherry-picked this PR to the wp/7.0 branch to get it included in the next release: 0a206c8 |
Co-authored-by: oandregal <oandregal@git.wordpress.org> Co-authored-by: ntsekouras <ntsekouras@git.wordpress.org> Co-authored-by: jasmussen <joen@git.wordpress.org> Co-authored-by: jameskoster <jameskoster@git.wordpress.org> Co-authored-by: harshbhonsle <harshbhonsle08@git.wordpress.org>
CI run: WordPress#11167. See #64595. --- I've included a log of the Gutenberg changes with the following command: git log --reverse --format="- %s" 022d8dd3d461f91b15c1f0410649d3ebb027207f..e499abfb843a43ac88455ca319220c5f181e1cf3 | sed 's|#\([0-9][0-9]*\)|https://github.com/WordPress/gutenberg/pull/\1|g; /github\.com\/WordPress\/gutenberg\/pull/!d' | pbcopy - Add documentation for contentRole and listView block supports (WordPress/gutenberg#75903) - Interactivity Router: fix back and forward navigation after refresh (WordPress/gutenberg#75927) - Real-time collaboration: Fix disconnect dialog on navigate (WordPress/gutenberg#75886) - Real Time Collab: Throttle syncing for inactive tabs. (WordPress/gutenberg#75843) - Components: Specify line-height to avoid inheriting default values (WordPress/gutenberg#75880) - Pattern Editing: Fix sibling blocks to edited pattern not being disabled (WordPress/gutenberg#75994) - Sync connector PHP behavior with Core backport changes (WordPress/gutenberg#75968) - Connectors: Avoid manual string concatenation (WordPress/gutenberg#75997) - DataForm: fix field label for panel (should not be uppercase) (WordPress/gutenberg#75944) - Views: add support for more overrides (all developer-defined config) (WordPress/gutenberg#75971) - Use homeUrl instead of siteUrl for link badge evaluations (WordPress/gutenberg#75978) - DataViews: Right-align `integer` and `number` fields (WordPress/gutenberg#75917) - Navigation Link: Compare internal links by host instead of origin (WordPress/gutenberg#76015) - Fix: Skip scaled image sideload for images below big image threshold (WordPress/gutenberg#75990) - Client side media cherry pick for 7.0 (WordPress/gutenberg#75998) - Show transform dropdown previews on focus as well as hover (WordPress/gutenberg#75940) (WordPress/gutenberg#75992) - RTC: Fix syncing of emoji / surrogate pairs (WordPress/gutenberg#76049) - [Real-time Collaboration] Fix sync issue on refresh (WordPress/gutenberg#76017) - Real-time collaboration: Improve disconnect dialog (WordPress/gutenberg#75970) - DataViews: Fix filter toggle flickering when there are locked or primary filters (WordPress/gutenberg#75913) (WordPress/gutenberg#76068) - Connectors: Dynamically register providers from WP AI Client registry (WordPress/gutenberg#76014) - PHP-only Blocks: Reflect bound attribute values in inspector controls (WordPress/gutenberg#76040) - Fix: Set quality and strip metadata in client-side image resize (WordPress/gutenberg#76029) - RTC: Prevent duplicate poll cycles (WordPress/gutenberg#76059) - RTC: Fix stale CRDT document persisted on save (WordPress/gutenberg#75975) - RTC: Disable multiple collaborators if meta boxes are present (WordPress/gutenberg#75939) - Directly inject styles in overlay to make styles stay consistently mounted (WordPress/gutenberg#75700) - Real-time collaboration: Fix comment syncing on site editor (WordPress/gutenberg#75746) - Real-time Collaboration: Bug fix for CRDT user selection and add tests (WordPress/gutenberg#75075) - RTC: Updates from backport PR (WordPress/gutenberg#75711) - RTC: Fix undefined array_first() call in sync storage (WordPress/gutenberg#75869) - RTC: Fix fallthrough for sync update switch statement (WordPress/gutenberg#76060) - Real-time collaboration: Remove block client IDs from Awareness, fix "Show Template" view (WordPress/gutenberg#75590) - RTC: Add session activity notifications (WordPress/gutenberg#76065) - Prevent non-reproducible Sass/CSS builds. (WordPress/gutenberg#76098) - Block toolbar and context menu: hide pattern actions in Revisions UI (WordPress/gutenberg#76066) - Try enabling style variation transforms for blocks in contentOnly mode (WordPress/gutenberg#75761) - Block toolbar: hide styles dropdown in Revisions UI (WordPress/gutenberg#76119) - Image block: fix lightbox srcset size (WordPress/gutenberg#76092) - Fix writing flow navigation for annotation style, or any other block with border radius (WordPress/gutenberg#76072) - Image: Hide 'Set as featured image' for in-editor revisions (WordPress/gutenberg#76123) - Connectors: Gate unavailable install actions behind install capability (WordPress/gutenberg#75980) - build: Exclude experimental pages from Core builds (WordPress/gutenberg#76038) - HTML & Shortcode: Disable viewport visibility support (WordPress/gutenberg#76138) - RTC: Verify client ID to avoid awareness mutation (WordPress/gutenberg#76056) - wp-build: Do not remove Core's default script modules registration (WordPress/gutenberg#75705) - wp-build: Deregister script modules before re-registering (WordPress/gutenberg#75909) - Remove `! function_exists()` checks from PHP templates (WordPress/gutenberg#76062) - Connectors: Update page identifier to options-connectors (WordPress/gutenberg#76156) - Connectors: Align init hook priorities with Core overrides (WordPress/gutenberg#76161) - Icon Block: Clean up selectors config (WordPress/gutenberg#75786) - Icons: Fix incorrect icon slug (WordPress/gutenberg#76165) - RTC: Enable RTC by default (WordPress/gutenberg#75739) - Rename and visibility modals: gate shortcuts behind canEditBlock to prevent triggering in revisions UI (WordPress/gutenberg#76168) - Fix: Block style variations not rendering in Site Editor Patterns page (WordPress/gutenberg#76122) - Client-side media processing: only use media upload provider when not in preview mode (WordPress/gutenberg#76124) - Notes: Disable for in-editor revisions (WordPress/gutenberg#76180) - Core Data: Support reading revision data in useEntityProp (fixes footnotes in revisions UI) (WordPress/gutenberg#76106) - Client-side media processing: Try plumbing invalidation to the block-editor's mediaUpload onSuccess callback (WordPress/gutenberg#76173) - Connectors: Improve responsive layout on small screens (WordPress/gutenberg#76186) - Interactivity API: Fix router initialization race condition on Safari/Firefox (WordPress/gutenberg#76053) (WordPress/gutenberg#76191) - Interactivity: Fix crypto.randomUUID crash in non-secure contexts (WordPress/gutenberg#76151) git-svn-id: https://develop.svn.wordpress.org/trunk@61843 602fd350-edb4-49c9-b593-d223f7449a82
CI run: WordPress/wordpress-develop#11167. See #64595. --- I've included a log of the Gutenberg changes with the following command: git log --reverse --format="- %s" 022d8dd3d461f91b15c1f0410649d3ebb027207f..e499abfb843a43ac88455ca319220c5f181e1cf3 | sed 's|#\([0-9][0-9]*\)|https://github.com/WordPress/gutenberg/pull/\1|g; /github\.com\/WordPress\/gutenberg\/pull/!d' | pbcopy - Add documentation for contentRole and listView block supports (WordPress/gutenberg#75903) - Interactivity Router: fix back and forward navigation after refresh (WordPress/gutenberg#75927) - Real-time collaboration: Fix disconnect dialog on navigate (WordPress/gutenberg#75886) - Real Time Collab: Throttle syncing for inactive tabs. (WordPress/gutenberg#75843) - Components: Specify line-height to avoid inheriting default values (WordPress/gutenberg#75880) - Pattern Editing: Fix sibling blocks to edited pattern not being disabled (WordPress/gutenberg#75994) - Sync connector PHP behavior with Core backport changes (WordPress/gutenberg#75968) - Connectors: Avoid manual string concatenation (WordPress/gutenberg#75997) - DataForm: fix field label for panel (should not be uppercase) (WordPress/gutenberg#75944) - Views: add support for more overrides (all developer-defined config) (WordPress/gutenberg#75971) - Use homeUrl instead of siteUrl for link badge evaluations (WordPress/gutenberg#75978) - DataViews: Right-align `integer` and `number` fields (WordPress/gutenberg#75917) - Navigation Link: Compare internal links by host instead of origin (WordPress/gutenberg#76015) - Fix: Skip scaled image sideload for images below big image threshold (WordPress/gutenberg#75990) - Client side media cherry pick for 7.0 (WordPress/gutenberg#75998) - Show transform dropdown previews on focus as well as hover (WordPress/gutenberg#75940) (WordPress/gutenberg#75992) - RTC: Fix syncing of emoji / surrogate pairs (WordPress/gutenberg#76049) - [Real-time Collaboration] Fix sync issue on refresh (WordPress/gutenberg#76017) - Real-time collaboration: Improve disconnect dialog (WordPress/gutenberg#75970) - DataViews: Fix filter toggle flickering when there are locked or primary filters (WordPress/gutenberg#75913) (WordPress/gutenberg#76068) - Connectors: Dynamically register providers from WP AI Client registry (WordPress/gutenberg#76014) - PHP-only Blocks: Reflect bound attribute values in inspector controls (WordPress/gutenberg#76040) - Fix: Set quality and strip metadata in client-side image resize (WordPress/gutenberg#76029) - RTC: Prevent duplicate poll cycles (WordPress/gutenberg#76059) - RTC: Fix stale CRDT document persisted on save (WordPress/gutenberg#75975) - RTC: Disable multiple collaborators if meta boxes are present (WordPress/gutenberg#75939) - Directly inject styles in overlay to make styles stay consistently mounted (WordPress/gutenberg#75700) - Real-time collaboration: Fix comment syncing on site editor (WordPress/gutenberg#75746) - Real-time Collaboration: Bug fix for CRDT user selection and add tests (WordPress/gutenberg#75075) - RTC: Updates from backport PR (WordPress/gutenberg#75711) - RTC: Fix undefined array_first() call in sync storage (WordPress/gutenberg#75869) - RTC: Fix fallthrough for sync update switch statement (WordPress/gutenberg#76060) - Real-time collaboration: Remove block client IDs from Awareness, fix "Show Template" view (WordPress/gutenberg#75590) - RTC: Add session activity notifications (WordPress/gutenberg#76065) - Prevent non-reproducible Sass/CSS builds. (WordPress/gutenberg#76098) - Block toolbar and context menu: hide pattern actions in Revisions UI (WordPress/gutenberg#76066) - Try enabling style variation transforms for blocks in contentOnly mode (WordPress/gutenberg#75761) - Block toolbar: hide styles dropdown in Revisions UI (WordPress/gutenberg#76119) - Image block: fix lightbox srcset size (WordPress/gutenberg#76092) - Fix writing flow navigation for annotation style, or any other block with border radius (WordPress/gutenberg#76072) - Image: Hide 'Set as featured image' for in-editor revisions (WordPress/gutenberg#76123) - Connectors: Gate unavailable install actions behind install capability (WordPress/gutenberg#75980) - build: Exclude experimental pages from Core builds (WordPress/gutenberg#76038) - HTML & Shortcode: Disable viewport visibility support (WordPress/gutenberg#76138) - RTC: Verify client ID to avoid awareness mutation (WordPress/gutenberg#76056) - wp-build: Do not remove Core's default script modules registration (WordPress/gutenberg#75705) - wp-build: Deregister script modules before re-registering (WordPress/gutenberg#75909) - Remove `! function_exists()` checks from PHP templates (WordPress/gutenberg#76062) - Connectors: Update page identifier to options-connectors (WordPress/gutenberg#76156) - Connectors: Align init hook priorities with Core overrides (WordPress/gutenberg#76161) - Icon Block: Clean up selectors config (WordPress/gutenberg#75786) - Icons: Fix incorrect icon slug (WordPress/gutenberg#76165) - RTC: Enable RTC by default (WordPress/gutenberg#75739) - Rename and visibility modals: gate shortcuts behind canEditBlock to prevent triggering in revisions UI (WordPress/gutenberg#76168) - Fix: Block style variations not rendering in Site Editor Patterns page (WordPress/gutenberg#76122) - Client-side media processing: only use media upload provider when not in preview mode (WordPress/gutenberg#76124) - Notes: Disable for in-editor revisions (WordPress/gutenberg#76180) - Core Data: Support reading revision data in useEntityProp (fixes footnotes in revisions UI) (WordPress/gutenberg#76106) - Client-side media processing: Try plumbing invalidation to the block-editor's mediaUpload onSuccess callback (WordPress/gutenberg#76173) - Connectors: Improve responsive layout on small screens (WordPress/gutenberg#76186) - Interactivity API: Fix router initialization race condition on Safari/Firefox (WordPress/gutenberg#76053) (WordPress/gutenberg#76191) - Interactivity: Fix crypto.randomUUID crash in non-secure contexts (WordPress/gutenberg#76151) Built from https://develop.svn.wordpress.org/trunk@61843 git-svn-id: http://core.svn.wordpress.org/trunk@61130 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Fixes #73365
What?
Right-align
numberorintegerfield types by default, while respecting explicit alignment.Why?
To provide better defaults and improve legibillity.
How?
Check the field type and set align end, if no align has been provided.
Testing Instructions