DataViews: Bootstrap Quick Edit #63600
Conversation
youknowriad
added
[Type] Feature
youknowriad
requested review from
oandregal,
jameskoster,
jasmussen,
ellatrix,
jorgefilipecosta and
ntsekouras
| @@ -201,6 +201,17 @@ export default function Layout( { route } ) { | |||
| </div> | |||
| ) } | |||
|
|
|||
| { ! isMobileViewport && areas.edit && ( | |||
There was a problem hiding this comment.
I added a new routable area to address this use-case. Routes in the site editor can decide to inject UI in the "edit" area.
| import Page from '../page'; | ||
| import usePostFields from '../post-fields'; | ||
|
|
||
| export function PostEdit( { postType, postId } ) { |
There was a problem hiding this comment.
We have PostList to render dataviews for a given post type.
This new component PostEdit is meant to edit the "form" or "inspector panel" of a given post type.
| ); | ||
| } | ||
|
|
||
| function usePostFields( viewType ) { |
There was a problem hiding this comment.
I extracted the "fields" from the "PostList" component to be able to reuse the same fields between "PostList" and "PostEdit". This is a strong principle that we should stick to.
viewType is not necessary IMO, I think ideally we should rework our fields to not depend on the view type. I left it for now though.
There was a problem hiding this comment.
Even though at this stage we share the fields between posts and pages, it would be good to have a mandatory argument of the postType passed here, no? It should be used with the future APIs with field registration etc..
There was a problem hiding this comment.
I don't know, as long as we don't need, I prefer to avoid it but I also think it's going to be useful very quickly.
There was a problem hiding this comment.
I think it should be similar to usePostActions and pass an object to be flexible with additions. We can handle later of course as it won't change anything now..
| header: __( 'Title' ), | ||
| id: 'title', | ||
| type: 'text', | ||
| getValue: ( { item } ) => |
There was a problem hiding this comment.
I updated this function to return the "raw" title instead of the "rendered" title. I think it's a good question to ask: what does this property correspond too. For me, it should correspond to the "raw" title". For the "rendered" values we have the render function to override this.
|
Size Change: +1.64 kB (+0.09%) Total Size: 1.76 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
That's a great start, thank you!
I had in mind that we would probably render the form in a dialog and could even make it draggable, but your approach of a new area looks good.
Having it as an action (Quick edit) makes the most sense to me. It might need some design input whether to make it a primary action and give a bit more prominence.
Some quick wins for this PR to feel a bit more polished would be:
- add a way to close the quick edit panel
- probably have a toast on save and have the button enabled only if we have changes
- Add a title in the panel to be clear which entity you are about to edit
How do we save changes.
I think having a centralized submit makes sense and it should be enough for this first iteration. I'm pretty sure we'll have more challenges when implementing other field types 😄
How do we organize the code base.
What do you mean by that?
Finally, some tweaks with the state will be needed to handle:
- Quick edit one item (panel opens)
- Quick edit different item
|
A couple of fundamental pieces from the design side:
I'll update #55101 with a more comprehensive prototype when I find time. |
|
To be honest, I'm very skeptical about "preview and edit" at the same time for list views but we can focus on table and grid for now if needed. Happy to update the PR. Please do share a prototype, that will help a lot. |
|
To be honest I think I'd rather lose the quick-edit functionality in List layout than the visible preview. The latter seems fundamental to that layout – without it there are too many overlaps with Table layout. Restricting quick edit to Grid / Table seems a good short term step. |
But we're not losing "preview", quick edit can be seen as a module, you do your edits and you return to the "preview". But I won't argue much here, I'm fine trying any approach. |
|
I added some mockups to #55101. |
|
I didn't disable it yet in list views, but we're getting closer to the prototypes. Can you check it? |
packages/dataviews/src/dataviews.tsx
Outdated
| @@ -75,6 +76,7 @@ export default function DataViews< Item >( { | |||
| selection: selectionProperty, | |||
| setSelection: setSelectionProperty, | |||
| onSelectionChange = defaultOnSelectionChange, | |||
| header, | |||
There was a problem hiding this comment.
This is a temporary prop until we address this issue #63646
|
The thing that feels weird to me is that clicking items in the table view actually bulk selects by default. I think it should unselect all the other items and selects the current one automatically. It should be different than clicking the checkbox. Maybe we could support ctrl + click for multi-select as well. |
I agree with this and with this PR especially it contributes to a quite confusing experience. I quickly tested and the status handling is still problematic when we change entities (table view for example). Another thought is if we don't have a quick edit post action, we still need to have checks about permissions etc to edit. A basic example would be the Based on this, at some cases the entity will not be editable. Should we have the button there disabled? Should we not render it and have the layout shift? |
I think we need some placeholder content for:
|
That works for me. Ctrl + click (and shift + click) would be nice enhancements. Small tweak: The toggle button should be
For now some centrally aligned text is probably adequate, e.g.: 
We can elaborate on this in follow-ups. |
|
We should probably show something when you bulk select too, but I'm not sure what the message should be. "Only one item can be edited at a time" is a bit underwhelming, but perhaps a necessary interim? |
youknowriad
force-pushed
the
update/bootstrap-quick-edit
branch
2 times, most recently
from
3cae411 to
d203618
Compare
youknowriad
force-pushed
the
update/bootstrap-quick-edit
branch
from
d203618 to
f2133b8
Compare
| onClick={ () => { | ||
| history.push( { | ||
| ...location.params, | ||
| quickEdit: quickEdit ? undefined : true, |
There was a problem hiding this comment.
I have checked and the quickEdit URL parameter seems to be cleared properly when switching screen (back to Design or into the Editor).
| @@ -608,13 +291,35 @@ export default function PostList( { postType } ) { | |||
| fields={ fields } | |||
| actions={ actions } | |||
| data={ records || EMPTY_ARRAY } | |||
| isLoading={ isLoadingMainEntities || isLoadingAuthors } | |||
| isLoading={ isLoadingMainEntities || isLoading } | |||
There was a problem hiding this comment.
Nit: isLoadingData || isLoadingFields.
Apparently not, it's working now :) I noticed a small regression though; in table layout the selected row no longer has blue accents. |
Never mind, this seems to be the case on trunk too. |
youknowriad
added
[Type] Experimental
|
A thought: we should probably make an effort to not refer to this as "Quick Edit" (which sounds like a named feature). In practice, it's just access to editing, or even bulk editing, and this action shouldn't officially have a name. |
|
Hm.. But it is an action.. Do you refer to the capitalization? If yes, this is probably because that's how we render it in wp-admin already. |
|
I think it's fine for us developing the software to call it this. To clarify, I'm saying this feature shouldn't officially have a name, or any user-facing name. E.g. when writing documentation, we should not refer to this as "Use the quick edit feature", it should be more in the vein of "If you're editing pages in the table view, you can show the details panel to edit additional properties". I see it's got "quick edit" in the tooltip, I wonder if we can change that to just "Show details"? |
We could do that yes. But do you think it's going to be clear that there we can actually edit? Another thing is that we are going to introduce this for |
|
The tooltip can be anything. Mainly I'd prefer "edit" used as a word rather than "Quick Edit" surfaced as a feature name. Describe the flow in a sentence rather than refer to a feature. |
|
Maybe something like 'View details' or 'Edit details' could work as the action name in List layout? In other layouts the tooltip could be 'Toggle details panel'? |
|
Sounds good to me! (Either.) |
|
Opened a quick PR to update the copy. |
Related to #55101 and #59745
What?
This PR bootstraps a panel for quick edit pages in the pages dataviews of the site editor. There's a lot of iterations and questions that needs answering but my goal for this initial PR is to set the "architecture" in place:
The following items will be left for other PRs: (I don't mind feedback on them but I know that they need work and would prefer to leave them separate)
Testing Instructions
1- Open the "pages" section of the site editor.
2- Click "quick edit" in the actions menu of a given page.
3- Notice that you can edit the title.