Skip to content

Added Edit button to the DataForm panel layout #73036

Closed
jimjasson wants to merge 7 commits into
WordPress:trunkfrom
jimjasson:tweak/add-edit-button-to-dataform-panels
Closed

Added Edit button to the DataForm panel layout #73036
jimjasson wants to merge 7 commits into
WordPress:trunkfrom
jimjasson:tweak/add-edit-button-to-dataform-panels

Conversation

@jimjasson
Copy link
Copy Markdown

What?

This PR improves the edit interaction for DataForm panel fields by replacing the clickable field value with a separate edit button that appears on hover.

Why?

The previous implementation made entire field values look and behave like buttons. This created an accessibility issue where screen readers would only announce "Edit [field name]" without revealing the current field value, leaving users without crucial context.

How?

  • Component changes: Refactored SummaryButton to render a wrapper div containing the summary content and a separate icon button, replacing the previous single-button implementation.
  • Styling: Added .dataforms-layouts-panel__summary-wrapper with hover states that fade the edit button in/out, plus @media (hover: none) to keep buttons always visible on touch devices.
  • Accessibility: Added useInstanceId to generate unique IDs for summary content, then linked each edit button to its summary using aria-describedby so screen readers announce both the action and current value.

Questions

  • For readOnly fields, I decided to not show the Edit button. Do you agree with this decision?
  • The labelPosition prop of SummaryButton was previously used to determine the variant of the button that overlayed the field value. I don't think that this is needed any longer. Do you agree?

Testing Instructions

  1. Run npm run storybook:dev.
  2. Navigate to DataForm examples.
  3. Validate that the values of panel fields are no longer clickable -- instead an edit button appears on hover.
  4. Validate that the edit flow works as it used to.
  5. For read-only fields, validate that you don't see an edit button on hover.
  6. For mobile/touch devices, validate that the edit icon is always visible.

Testing Instructions for Keyboard

  1. Run npm run storybook:dev.
  2. Navigate to DataForm examples.
  3. Click on one of the examples that contain a panel field, e.g the Customer card.
  4. Use the tab key to navigate the page.
  5. Validate that the edit buttons appear when focused by the tab.
  6. Validate that by clicking enter the edit panel opens up.
Screen.Capture.on.2025-11-06.at.10-22-30.mp4

Screenshots or screencast

Before After
Markup on 2025-11-06 at 10:27:19 Markup on 2025-11-06 at 10:27:41

Desktop experience

Screen.Capture.on.2025-11-06.at.10-30-00.mp4

Mobile experience

Screen.Capture.on.2025-11-06.at.10-31-20.mp4

@jimjasson jimjasson requested a review from oandregal as a code owner November 6, 2025 08:33
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 6, 2025

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 props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @poligilad-auto.

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.

Unlinked contributors: poligilad-auto.

Co-authored-by: jimjasson <jasonkytros@git.wordpress.org>
Co-authored-by: oandregal <oandregal@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@github-actions github-actions Bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Nov 6, 2025
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Nov 6, 2025

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @jimjasson! 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.

@oandregal oandregal added [Type] Enhancement A suggestion for improvement. [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond labels Nov 6, 2025
@oandregal oandregal added the Needs Design Feedback Needs general design feedback. label Nov 13, 2025
@oandregal
Copy link
Copy Markdown
Member

oandregal commented Nov 13, 2025

  • Design-wise, it is difficult to tell apart the label from the actual value. I'm going to summon design folks for this:
Screen.Recording.2025-11-13.at.16.37.49.mov
  • For empty value, the edit affordance is not super clear (same issue we have now):
Screen.Recording.2025-11-13.at.16.52.04.mov
  • Additionally, we need to figure out how custom Edit controls work in this scenario.

This is how QuickEdit behaves in Site Editor > Pages (need to have enabled the experiment in Gutenberg > Experiments). Note how the template field renders its own dropdown with one or two actions before opening the modal. We don't support that level of interaction right now, so the field built its own custom Edit component.

Screen.Recording.2025-11-13.at.17.06.11.mov

@poligilad-auto
Copy link
Copy Markdown

Design-wise, it is difficult to tell apart the label from the actual value. I'm going to summon design folks for this:

I don't know if we can align on this for all DataForm fields (maybe not the panel view), but we prefer the label to be the same styling as input fields (bold) but with gray color instead of black to indicate that this is read-only.
This creates a strong visual link between fully editable and read-only fields, especially useful for mixed forms.

image image

Editable read-only fields will have bolder text, similar visually to buttons. It's a little subtle, but it was the latest approach.

image

We also suggested a background color on hover to emphasize the editable fields.

image

For empty value, the edit affordance is not super clear (same issue we have now):

Maybe we can solve this with copy, like this. "Enter value" for example, with gray text to indicate that it's empty.

image

@jameskoster any other thoughts?

@jameskoster
Copy link
Copy Markdown
Contributor

In terms of the general UX a dedicated edit button makes sense and seems like a nice a11y enhancement, but I think the entire field should also be clickable to trigger the modal/popover. Additionally the pencil button should appear when hovering the whole field to address Andrés first feedback. I made a prototype here to demonstrate the interaction.

Otherwise @poligilad-auto's feedback sounds good. I could potentially see un-editable field values being gray too to further distinguish, but it's not a strong feeling. Here's a diagram to illustrate in case it's useful:

Screenshot 2025-11-21 at 16 51 48

Note how the template field renders its own dropdown with one or two actions before opening the modal.

Is this definitely something we want to support moving forwards? Actions potentially feel a bit out of scope for DataForm and things can get a bit complex when actions are conditionally available (IE what do we do after the template has been reset, show a menu containing one item?). In this particular example swapping the template seems fine, but the 'reset' action should arguably live elsewhere, grouped with other contextual actions like "Rename", "Trash", and so on.

If we do want to support this then maybe we replace the pencil icon with a more icon and show it when hovering the field. But in this case only the icon button would be interactive, not the entire field.

Screenshot 2025-11-21 at 17 03 00

@oandregal
Copy link
Copy Markdown
Member

Is this definitely something we want to support moving forwards? Actions potentially feel a bit out of scope for DataForm and things can get a bit complex when actions are conditionally available (IE what do we do after the template has been reset, show a menu containing one item?). In this particular example swapping the template seems fine, but the 'reset' action should arguably live elsewhere, grouped with other contextual actions like "Rename", "Trash", and so on.

I also agree that actions in DataForm are out of scope for now, so this means that:

  • the template field opens a modal to change the template
  • there's a new "reset template" action that is visible in dataviews, and also accesible via the form title

@jimjasson this means we need to update how the template field works in Site Editor > Pages. It may be worth addressing this as a separate PR, as preparation for this one.

@jimjasson
Copy link
Copy Markdown
Author

Thank you for the feedback @poligilad-auto and @jameskoster !

A couple of clarification questions on the designs you proposed, and especially this one:

image
  • When hovering over a read-only value, should the row be highlighted similarly to editable fields?
  • In a wide screen, where should the edit button show up? Right next to the label/value or at the right end of the row, as shown here?
  • In mobile screens, should a) the edit icon be always visible and b) no highlight on click/hover (similar to DataViews)?

@poligilad-auto
Copy link
Copy Markdown

Thanks @jimjasson!

When hovering over a read-only value, should the row be highlighted similarly to editable fields?

You mean "Uneditable" fields, correct? No, it's shouldn't be highlighted.

In a wide screen, where should the edit button show up? Right next to the label/value or at the right end of the row, as shown here?

At the end of the label, like here.
image

In mobile screens, should a) the edit icon be always visible and b) no highlight on click/hover (similar to DataViews)?

Yes to both, makes sense to align this behavior with DataViews.

Jay is AFK this week, but I think he'll agree with with me 🙂

@jimjasson
Copy link
Copy Markdown
Author

jimjasson commented Nov 25, 2025

Regarding empty buttons:

Maybe we can solve this with copy, like this. "Enter value" for example, with gray text to indicate that it's empty.

I think that it should be up to each field to decide how it wants to handle its empty state and DataForm shouldn't provide a default one.

For example, for some fields it might be more appropriate to have a -, for others a placeholder text etc. This can be controlled via the field's render function.

What do you think @oandregal ? Any chance that the issue with the empty value in the Storybook can be fixed by adding a render function to the Title field?

What led me to that thought is that the render function is being called for each field: https://github.com/WordPress/gutenberg/blob/a4de953b608164a20889b3a5090b049b967988e0/packages/dataviews/src/dataform-layouts/panel/summary-button.tsx#L81C7-L81C26. This makes sense because, for example, for boolean fields, the label of each option should be rendered and not the value (which will be true/false). If a render function is required to handle this situation, then perhaps a render function should also handle empty values.

- Enabled clicking entire field row to trigger edit action
- Revelaed edit button on row hover (previously only on value hover)
- Increased field value font-weight to 500 for better hierarchy
- Styled read-only fields with muted gray-700 color
- Implemented smart click detection to avoid triggering on interactive elements
- Added `is-read-only` class to read only fields
- Improved accessibility for read-only buttons
@jimjasson
Copy link
Copy Markdown
Author

Thanks @poligilad-auto!

At the end of the label, like here.

What happens in the case where the label is hidden? labelPosition=none?

@jimjasson jimjasson marked this pull request as draft November 25, 2025 10:01
@jimjasson
Copy link
Copy Markdown
Author

Converting to draft, as this is not ready for review yet.

@poligilad-auto
Copy link
Copy Markdown

What happens in the case where the label is hidden? labelPosition=none?

Oh, that's a hard one. Can we position it at the end of the row in that case? 😅
I don't think this will be a common use case.

@jameskoster
Copy link
Copy Markdown
Contributor

Agree it can go at the end of the row when the label is hidden. Here's an updated diagram:

Screenshot 2025-12-02 at 15 27 02

It's not the best, but I'm not sure there's a perfect solution to this config. Like Poli says it should be relatively rare so I think this is okay.

@jimjasson
Copy link
Copy Markdown
Author

It may be worth addressing this as a separate PR, as preparation for this one.

@oandregal Here it is: #73880

@verofasulo
Copy link
Copy Markdown

verofasulo commented Dec 19, 2025

@jameskoster sharing my feedback here as well 🙂

Visually greying out fields introduces hierarchy issues and risks making elements appear disabled.

You explored several approaches, and associating a small pencil icon with editable fields still feels like the strongest option to me. Did we discard that direction for any particular reason?

@jameskoster
Copy link
Copy Markdown
Contributor

@verofasulo do you mean something like this?

Screenshot 2025-12-19 at 15 37 57

I think the main feedback I received about this approach is that in 99% of cases "it's going to be a lot of pencils" 😅 I think that's valid given most forms won't have many non-editable fields. We should optimise for the more common use cases.

appear disabled

I mentioned this elsewhere, but non-editable fields basically are disabled if we think about the form context.

I think the hierarchy point can potentially be addressed by grouping the fields based on whether they're editable or not. Obviously this is a consumer/implementation point though.

@verofasulo
Copy link
Copy Markdown

I meant this one 🙂

image

@jameskoster
Copy link
Copy Markdown
Contributor

The placement of the icons in that one won't work because fields don't always have labels. We might be able to try the previous mockup with a smaller pencil icon, but it's still a lot of pencils 🙈

@oandregal
Copy link
Copy Markdown
Member

@jameskoster @verofasulo @poligilad-auto what are the next steps on this one?

The issue with empty values has been mitigated with some improvements, though it's still far from being good enough:

Screen.Recording.2026-01-09.at.09.45.01.mov

From what we have now, these are the design changes I see discussed:

  1. Make the whole field clickable, as opposed to only the field's value as of now.
  2. Editable vs. read-only fields:
    1. icon is visible for editable fields at all times
    2. icon is visible for editable fields only on hover + read-only fields are gray-out

Can we ship 1 (making the entire field clickable) as a first step? That's a good improvement that appears to have agreement. In the meanwhile, design can iterate on the second point.

@jameskoster
Copy link
Copy Markdown
Contributor

Can we ship 1 (making the entire field clickable) as a first step? That's a good improvement that appears to have agreement. In the meanwhile, design can iterate on the second point.

Yes I think that's the way to go. We're at a bit of an impasse with the second point... I don't think we should show the icon permanently as it's going to result in a lot of superfluous repetition in many cases. The document inspector for example:

Screenshot 2026-01-09 at 13 34 45

@jasmussen
Copy link
Copy Markdown
Contributor

Let's unstick this. I'll support what André suggested:

Can we ship 1 (making the entire field clickable) as a first step? That's a good improvement that appears to have agreement. In the meanwhile, design can iterate on the second point.

It addresses the immediate point, and does not preclude future iteration on edit affordances. Jay has explained sufficiently why applying it everywhere will make the editor as it exists today worse. There are valid contexts for DataForm to have separate edit affordances, but that needs iteration that considers these details across the range of DataForm contexts.

@poligilad-auto
Copy link
Copy Markdown

Maybe we can take a similar approach overall, but optimize it differently in each space: show the pencil icon in the admin areas, and omit it in the editor. We can still use similar hover states in both contexts. Here's a rough comparison, with no hover states:

image image

@jasmussen
Copy link
Copy Markdown
Contributor

I'd tend to agree that can be the iteration and endstate. The challenge is that this is one component, so it needs a systematic approach to what makes the edit icon show up: is it based on density? Is it based on a mix of componentry? And that needs to then be codified, which is the only part that's tricky. That's why I'd recommend shipping this as TWO PRs. One that addresses the urgent need as proposed, and one followup as you propose.

@jameskoster
Copy link
Copy Markdown
Contributor

We should probably continue this discussion in a separate issue, but it may be worth exploring an approach that only distinguishes between editable and non-editable fields when they coexist within the same form. That would allow examples like the Inspector to remain pencil-less.

@oandregal
Copy link
Copy Markdown
Member

I'm preparing a separate PR to address the first step as outlined here at #75290 It makes the whole field clickable.

The second step (different edit modes) will be a follow-up PR to be prepared when that lands.

@oandregal
Copy link
Copy Markdown
Member

#75290 has landed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository Needs Design Feedback Needs general design feedback. [Type] Enhancement A suggestion for improvement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants