Skip to content
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

Open
wants to merge 5 commits into
base: trunk
Choose a base branch
from

Conversation

Souptik2001
Copy link

@Souptik2001 Souptik2001 commented May 25, 2024

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.

  • Open the site-editor.
  • Go to "Pages".
  • Now switch the active view by clicking on the left sidebar.
  • As you change the views you will see that the subtitle under the title "Pages" also changes accordingly.

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

Signed-off-by: Souptik Datta <souptikdatta2001@gmail.com>
Copy link

github-actions bot commented May 25, 2024

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.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: Souptik2001 <souptik@git.wordpress.org>
Co-authored-by: jameskoster <jameskoster@git.wordpress.org>
Co-authored-by: t-hamano <wildworks@git.wordpress.org>
Co-authored-by: oandregal <oandregal@git.wordpress.org>

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

Copy link

👋 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.

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label May 25, 2024
@jameskoster
Copy link
Contributor

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 jameskoster requested review from youknowriad, ntsekouras and a team May 28, 2024 09:27
@youknowriad youknowriad requested a review from jameskoster May 28, 2024 10:03
@Souptik2001
Copy link
Author

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?

@jameskoster
Copy link
Contributor

Just taking a confirmation, that should I change it?

I think so, yes! Thank you.

@Souptik2001
Copy link
Author

Thanks @jameskoster for confirming!

I have made the update! Thanks!

@jameskoster
Copy link
Contributor

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>
@Souptik2001 Souptik2001 changed the title DataViews Pages: Add frame subtitle for active view DataViews Pages: Dynamic Frame title as per active view for Pages and Templates editor pages May 29, 2024
@Souptik2001
Copy link
Author

Thanks @jameskoster! Yes we can also update templates to have the same behaviour! 👍

I have made that change!
I have also updated the video in the description, you can check now for both page and template.
Also I have updated the PR title to make it more appropriate.

@jameskoster jameskoster added [Type] Enhancement A suggestion for improvement. [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond labels May 30, 2024
@jameskoster
Copy link
Contributor

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:

Screenshot 2024-05-30 at 11 09 31

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:

Screenshot 2024-05-30 at 11 16 50

I'm keen to gather thoughts from others on that cc @WordPress/gutenberg-design.

@t-hamano
Copy link
Contributor

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:

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.

@jameskoster
Copy link
Contributor

@t-hamano that sounds like the first point in #60468. I think the question is whether or not we consider that a blocker to this PR.

@t-hamano
Copy link
Contributor

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.

@Souptik2001
Copy link
Author

@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. 🙂
Thanks!

@jameskoster
Copy link
Contributor

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 👍

@Souptik2001
Copy link
Author

Thanks @jameskoster! I have raised a PR for first three points of #60468 !

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 [Type] Enhancement A suggestion for improvement.
Projects
Status: In Dev
Development

Successfully merging this pull request may close these issues.

DataViews: Frame title should match current view
3 participants