-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
DataViews Pages: Dynamic Frame title as per active view for Pages and Templates editor pages #61983
base: trunk
Are you sure you want to change the base?
Conversation
Signed-off-by: Souptik Datta <souptikdatta2001@gmail.com>
…le-for-active-view
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. |
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @Souptik2001! 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. |
Thank you for opening the PR :) I think it probably makes sense to use the main title, rather than a subtitle, at least initially. This would align with the Pattern management page, and leave the subtitle free for additional context to be added later if needed. What do you think? |
@jameskoster Yes I align with your thought! That would be better! I made it like this as per this comment - #56241 (comment) Just taking a confirmation, that should I change it? |
I think so, yes! Thank you. |
Signed-off-by: Souptik Datta <souptikdatta2001@gmail.com>
Thanks @jameskoster for confirming! I have made the update! Thanks! |
This works well for me. I think it would make sense to apply the same treatment to the templates page. What do you think? |
Signed-off-by: Souptik Datta <souptikdatta2001@gmail.com>
Thanks @jameskoster! Yes we can also update templates to have the same behaviour! 👍 I have made that change! |
Thanks for updating the Templates page :) There is a quirk (as I think you mentioned on the issue): if you reset the filters on a bundled page, the title can be misleading. E.g. Go to trashed pages, remove the status filter, and you're no longer viewing trashed pages: This will be fixed when #60468 is addressed. So the question here is whether we should include this change in the upcoming 6.6 release, or whether we should consider #60468 a blocker. The main argument for including it is to establish alignment with the Patterns page, where the frame title matches the menu item: I'm keen to gather thoughts from others on that cc @WordPress/gutenberg-design. |
To avoid this discrepancy, how about excluding the sidebar items from the data view filter? For example, on a page, if you select "Draft" in the sidebar, the data view will only list posts with a Draft status. However, it does not add any filters and we cannot reset the post status in the DataViews. |
Ah, I didn't read that issue thoroughly enough 😅 Personally, I would lean towards not including this PR in 6.6 and moving the discussion forward in #60468. |
@jameskoster @t-hamano I can get that task rolling. 🙂 If I understood correctly, for this first version we just want a visual feedback of whether the view is editor or not right(as per the design is provided)? Like the "Save view" feature, etc. will be introduced later, correct? If you give the go-ahead I can work on that one. 🙂 |
Yup, I think the first three tasks in #60468 are related, and can potentially be tackled in a single PR. The fourth and fifth tasks can come later 👍 |
Thanks @jameskoster! I have raised a PR for first three points of #60468 ! |
What?
In DataList views for pages, add a subtitle to display the activeView.
Why?
Closes: #56199
This requirement is first mentioned in the above issue.
But a more refined requirement can be found in this comment - #56241 (comment)
Just looking at the whitespace of the DataList Views of pages, it might be a little unclear what activeView you are on. That's why we can add a sub-title below the title "Pages" to display the activeView over there.
How?
This PR adds a sub-title for DataViews screen for pages, which displays the activeView.
Testing Instructions
Please see the below screencast section or refer the steps below.
Testing Instructions for Keyboard
N/A
Screenshots or screencast
Screen.Recording.2024-05-30.at.12.12.17.AM.mp4
OLD VIDEO:
Screen.Recording.2024-05-25.at.7.24.33.PM.mov