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

[issues] toggle filters btn behavior #4683

Merged
merged 3 commits into from
Nov 24, 2022
Merged

Conversation

philli-m
Copy link
Contributor

@philli-m philli-m commented Nov 14, 2022

@fuzzylogic2000 or @goapunk I am very confused with this, so the behavior we wish can be seen in the filter toggle for the comments, so I copied that code to make a component, planning to use it in a4 but wanted to test it first and even with exact classes it still shows being active after filters are opened, even though in comments it does not and I have wasted too much time on this by now so any ideas would be great!?

@philli-m philli-m force-pushed the pm-2022-11-toggle-filters branch from 018f951 to 1978b4e Compare November 14, 2022 16:10
@fuzzylogic2000
Copy link
Contributor

My guess its' that it is the prevent default in the filter button click? https://github.com/liqd/adhocracy4/blob/main/adhocracy4/comments_async/static/comments_async/comment_box.jsx#L261

@philli-m philli-m force-pushed the pm-2022-11-toggle-filters branch from 1978b4e to b81c69c Compare November 15, 2022 09:35
@philli-m philli-m force-pushed the pm-2022-11-toggle-filters branch from b81c69c to 87f4474 Compare November 22, 2022 16:57
@philli-m philli-m changed the title WIP [issues] toggle filters btn behavior [issues] toggle filters btn behavior Nov 22, 2022
@philli-m
Copy link
Contributor Author

philli-m commented Nov 22, 2022

I actually did some other fixing, the button focus is not that same as on the comments but I read that we shouldn't have btn randomly loose focus so will leave that, still think having a separate component is good though as will add to a4 next time we do stuff there

Copy link
Contributor

@fuzzylogic2000 fuzzylogic2000 left a comment

Choose a reason for hiding this comment

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

Very nice! But the tests are failing

@philli-m philli-m force-pushed the pm-2022-11-toggle-filters branch from 87f4474 to bc54ac5 Compare November 23, 2022 17:24
@philli-m
Copy link
Contributor Author

when fixing tests I realised that we shouldn't show the search string when we are ordering list so fixed that

@philli-m philli-m force-pushed the pm-2022-11-toggle-filters branch from bc54ac5 to 5f07dc1 Compare November 24, 2022 06:56
Copy link
Contributor

@fuzzylogic2000 fuzzylogic2000 left a comment

Choose a reason for hiding this comment

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

Cool!

@fuzzylogic2000 fuzzylogic2000 merged commit 417cdf7 into main Nov 24, 2022
@fuzzylogic2000 fuzzylogic2000 deleted the pm-2022-11-toggle-filters branch November 24, 2022 08:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants