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

Advanced event filters #22

Merged
merged 7 commits into from
Feb 9, 2024
Merged

Advanced event filters #22

merged 7 commits into from
Feb 9, 2024

Conversation

foysalit
Copy link
Contributor

@foysalit foysalit commented Feb 2, 2024

Depends on: bluesky-social/atproto#2124

advanced_event_filters_sm.mov

Demo

This PR changes the way moderation events are filtered/configured. Instead of the basic dropdown filter for specific event types, moderators now have a fully configurable filter panel that allows them to use all available filters/params on the queryModerationEvents endpoint.

Copy link

render bot commented Feb 2, 2024

Copy link

render bot commented Feb 2, 2024

@@ -427,7 +427,7 @@ function Form(
}
}
`}</style>
<div className="flex overflow-y-auto scrollable-container">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

make room for the ring around the Configure button in the mod event list header.

@foysalit foysalit changed the base branch from main to page-title February 2, 2024 14:23
@@ -37,7 +37,7 @@ export function FullScreenActionPanel(props: {
leaveFrom="opacity-100 scale-100"
leaveTo="opacity-0 scale-95"
>
<Dialog.Panel className="max-w-screen-lg w-full sm:w-5/6 h-full md:max-h-3/4 md:my-12 align-bottom bg-white rounded-lg text-left sm:overflow-hidden shadow-xl transform transition-all sm:align-middle flex">
<Dialog.Panel className="max-w-screen-xl w-full sm:w-5/6 h-full md:max-h-3/4 md:my-12 align-bottom bg-white rounded-lg text-left sm:overflow-hidden shadow-xl transform transition-all sm:align-middle flex">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Make room for the filter panel's content in the event list

package.json Outdated
@@ -17,7 +17,7 @@
"e2e:run": "$(yarn bin)/cypress run"
},
"dependencies": {
"@atproto/api": "0.9.2",
"@atproto/api": "link:../atproto/packages/api/dist",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will clean this up before merging

onKeyDown={(e) => {
if (e.key === 'Enter') {
e.preventDefault() // make sure we don't submit the form
e.currentTarget.click() // simulate a click on the input
Copy link
Collaborator

Choose a reason for hiding this comment

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

Having trouble picturing it—what happens in the ui when the user presses enter?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hmm... we're not using this inside a form so this is not necessary so I'm guessing it was just copy-paste thingy. getting rid of it.

createdBy: undefined,
subject: undefined,
oldestFirst: false,
createdBefore: new Date().toISOString().split('.')[0],
Copy link
Collaborator

Choose a reason for hiding this comment

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

This little snippet is used a few places. It's hard to picture exactly what it does, or where that date format is needed. Maybe just pulling it into a helper with a short comment would clear things up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yep thanks!

}

useEffect(() => {
if (props.subject !== listState.subject) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

No biggie either way, but these checks for no-ops might make sense to move into the reducer so that dispatch() can be called more freely.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yeah that works!


const EMPTY_ARR = []
type SelectProps = React.ComponentProps<typeof Select>

// TODO: Probably redundant
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is just cleaning up unused code.

Copy link
Collaborator

@bnewbold bnewbold left a comment

Choose a reason for hiding this comment

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

generally good, a few functional things:

  • if you select no event types, I think you get all, or generic? seems like it should be empty
  • it doesn't seem like the report, by-type works. I tried "spam" and "appeals" in in both cases had all sorts of reports mixed in

otherwise looks good!

@foysalit
Copy link
Contributor Author

foysalit commented Feb 7, 2024

if you select no event types, I think you get all, or generic? seems like it should be empty

I'm not sure if there's a use-case for not selecting any type? the result would always be empty list, right? may be the UI can prevent unselecting all items from the list to avoid this confusion?

it doesn't seem like the report, by-type works. I tried "spam" and "appeals" in in both cases had all sorts of reports mixed in

Seems like there's an issue with entryway PDS missing lexicons. Devin said he will sync it up later today. I'll put a hold on this until that's done.

@bnewbold

@foysalit
Copy link
Contributor Author

foysalit commented Feb 9, 2024

Alright after Devin sent out the lexicon changes to entryway, all filters are working! Going to merge now.

@foysalit foysalit merged commit 89db813 into page-title Feb 9, 2024
3 checks passed
@matthieusieben matthieusieben deleted the advanced-event-filters branch November 15, 2024 14:54
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.

3 participants