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

Adding a filter from the column-header-menu should open the filter panel #64683

Open
wants to merge 1 commit into
base: trunk
Choose a base branch
from

Conversation

imrraaj
Copy link
Contributor

@imrraaj imrraaj commented Aug 21, 2024

What?

The fix shows the filter component when selected via column headers

Why?

Fixes #64612

How?

The initial state for isShowingFilter is set to true, so when the filter option is selected in column header the filter component renders just like when we select it using dropdown menu icon

const [ isShowingFilter, setIsShowingFilter ] = useState< boolean >( () =>
( filters || [] ).some( ( filter ) => filter.isPrimary )
);

to

 const [ isShowingFilter, setIsShowingFilter ] = useState< boolean >( true );

Testing Instructions

  1. Clone the repo
  2. run npm install and npm run storybook:dev
  3. go to dataview component and select categories column header
  4. select the Add filter option
  5. See the render filter component.

Screenshots or screencast

Before

Screen.Recording.2024-08-21.at.7.32.56.PM.mov

After

Screen.Recording.2024-08-21.at.7.30.31.PM.mov

@jameskoster jameskoster added [Type] Bug An existing feature does not function as intended [Feature] DataViews Work surrounding upgrading and evolving views in the site editor and beyond labels Aug 21, 2024
@youknowriad
Copy link
Contributor

Do we think that having two places to add filters is a good idea? Why can't we just remove the filters from the table headers (no other view outside table allows this inline filtering)

@jameskoster
Copy link
Contributor

I think it's quite handy, as are the other actions in that menu (re-ordering, hiding).

Other actions like pinning (#64679) could be contained here in the future. Thinking about custom fields you might also be able to rename, or go and edit other field properties from this menu.

@imrraaj imrraaj marked this pull request as ready for review August 22, 2024 05:21
Copy link

github-actions bot commented Aug 22, 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.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @[email protected].

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: [email protected].

Co-authored-by: jorgefilipecosta <[email protected]>
Co-authored-by: youknowriad <[email protected]>
Co-authored-by: jameskoster <[email protected]>
Co-authored-by: imrraaj <[email protected]>
Co-authored-by: oandregal <[email protected]>

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

@imrraaj
Copy link
Contributor Author

imrraaj commented Aug 26, 2024

Hey @jameskoster, Can you please review the PR?

@jameskoster
Copy link
Contributor

@imrraaj this one needs a code review from someone like @youknowriad or @jorgefilipecosta.

@oandregal
Copy link
Member

Interestingly, it works in the storybook (where DataViews is a standalone component), so I wonder if there's some edit-site code that is preventing this from working in the site editor:

Gravacao.do.ecra.2024-09-02.as.12.29.04.mov

Copy link
Member

@jorgefilipecosta jorgefilipecosta left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR causes an unexpected change. On templates when we select a source like an author or a theme with want the filters to be hidden by default:
Screenshot 2024-10-07 at 12 10 31

On this PR the filters become visible.

@jorgefilipecosta
Copy link
Member

@jameskoster, would the change I described above be something we are ok with? Or is it something we want to avoid?

@jameskoster
Copy link
Contributor

@jorgefilipecosta Ideally the bundled views all behave the same in this respect.

  • In the Pages page filters are hidden entirely when browsing by status.
  • In the Patterns page filters are hidden entirely when browsing by category.
  • In the Templates page, filters are hidden by default but still accessible when browsing by author.

As we can see Templates are the odd one out. But this is an issue on trunk, so we don't necessarily have to fix it here. What do you think?

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 [Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Adding a filter from the column-header-menu should open the filter panel
5 participants