Fix: Support setValue in fields to handle nested data updates in DataViews filters and forms #70989
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 Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @Jeelislive. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. 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. |
github-actions
bot
added
the
First-time Contributor
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @Jeelislive! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
t-hamano
added
[Type] Bug
| @@ -138,7 +147,7 @@ export default function FormRegularField< Item >( { | |||
| <fieldDefinition.Edit | |||
| data={ data } | |||
| field={ fieldDefinition } | |||
| onChange={ onChange } | |||
| onChange={ handleFieldChange } | |||
There was a problem hiding this comment.
Instead of handling this at the layout-level, it should be handled at the Edit (controls) level. For example, in this control. Fields can provide a custom Edit function (control) and when they do so it's their responsibility to implement this.
| @@ -15,6 +15,10 @@ import type { useFocusOnMount } from '@wordpress/compose'; | |||
|
|
|||
| export type SortDirection = 'asc' | 'desc'; | |||
|
|
|||
| /** | |||
| * Callback used to set the value of the field in the item. | ||
| * Defaults to `{ ...item, [field.id]: value }`. | ||
| */ | ||
| setValue?: ( args: { item: Item; value: any } ) => Item; |
There was a problem hiding this comment.
It'd be nice to rename value to newValue (same in all other lines).
There was a problem hiding this comment.
Hey @Jeelislive thanks for the contribution. The direction sounds good, there are some things to follow-up on, would you be available to do so in the next days?
Some other comments:
- There are a few spacing changes that shouldn't be there. It looks like you don't have the local git hooks set up. Take a look at this guide.
- Can you add tests and/or storybook example for this code? It's an important aspect, and we want to make sure all cases are well covered and that future changes don't break this either.
| return; | ||
| } | ||
| // Use setValue to extract the new value for this field | ||
| let nextValue; |
There was a problem hiding this comment.
At this point, all fields are normalized, and so they'll have a proper setValue. Additionally, if we handle this at the controls-level (see comment below), updatedData will be fine already. The only thing we'd need to do is making sure nextValue is calculated from getValue.
|
@Jeelislive is sounds like you don't have your GitHub account associated to your WordPress.org profile. Take a look at this guide so you receive props when this lands. |
|
@oandregal Let me work on these updates, and I’ll circle back here once they’re ready! |
fixes an issue where user-input filters and edit forms in DataViews didn’t work correctly for fields that use nested values with a getValue function (e.g., title.raw).
What’s changed:
setValuefunction in field definitions.setValuewhen provided.This keeps field logic (get/set) inside the field definition and avoids breaking data formats.
Fixes #70957