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 tag filters #266

Merged
merged 11 commits into from
Jan 8, 2025
Merged

✨ Advanced tag filters #266

merged 11 commits into from
Jan 8, 2025

Conversation

foysalit
Copy link
Contributor

@foysalit foysalit commented Dec 31, 2024

This PR changes the current filter panel with a more powerful tag filter builder.

This interface allows moderators to filter the queue in a more useful ways like

  1. Only show accounts that were reported for spam and no other reason
  2. Only show english posts with image that were not reported for spam
Screenshot 2024-12-31 at 15 30 35

Light mode, showing the exclude tag builder

Screenshot 2024-12-31 at 15 31 02

Dark mode, showing the record type filter

@@ -491,7 +492,13 @@ function Form(
submitForm()
}
useKeyPressEvent('c', safeKeyHandler(onCancel))
useKeyPressEvent('s', safeKeyHandler(submitForm))
useKeyPressEvent(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unrelated to the PR. fixes #265

safeKeyHandler(() => {
setModEventType(MOD_EVENTS.TAKEDOWN)
}),
canTakedown
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixes #244

<LabelList className="-ml-1 flex-wrap">
{subjectStatus.tags.map((tag) => {
<LabelList className="-ml-1 flex-wrap gap-1">
{subjectStatus.tags.sort().map((tag) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes having the lang tags being placed in random places.

Copy link
Contributor

Choose a reason for hiding this comment

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

Note that .sort() will sort the input array in place. I don't know if that's fine or if a copy should be made before sorting ?

@@ -29,7 +29,7 @@ const Timer = () => {
}, 1000)

return (
<div className="flex flex-row justify-center py-4">
<div className="flex flex-row justify-center py-4 dark:text-gray-200">
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes #152

@@ -27,12 +27,12 @@ export function SetupModal({
className="mx-auto h-20 w-auto"
title="Icon from Flaticon: https://www.flaticon.com/free-icons/lifeguard-tower"
src="/img/logo-colorful.png"
alt="Ozone - Bluesky Admin"
alt="Ozone - ATProto Moderation Service"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Addresses #216

}

const selectClassNames = {
tagItemIconContainer:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These classes are mostly copy pasted from the library itself because the lib doesn't append/overwrite classes and instead, replaces entirely based on customized config so in order to maintain current styling AND add dark mode, we need to bring over all classes the lib uses and add our own dark mode classes.

@@ -640,7 +640,7 @@ function Details({
</LabelList>
</DataField>
<DataField label="Tags">
<LabelList>
<LabelList className='flex-wrap gap-1'>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes tags spilling outside of view port on the profile view page.

@@ -13,7 +13,6 @@ export const getStaticActions = ({
{
id: 'quick-action-modal',
name: 'Open Quick Action Panel',
shortcut: ['q'],
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we never use these global shortcuts and they break some other shortcuts we use contextually in various panels so getting rid of these entirely.
fixes #265

@@ -40,6 +40,7 @@
"react-dom": "18.2.0",
"react-dropzone": "^14.3.5",
"react-json-view": "1.21.3",
"react-tailwindcss-select": "^1.8.5",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

we were using this package before and made OS contributions to it as well. we could replicate some of its behavior via combobox but that requires a lot of work so opting in for this one again.

Copy link
Contributor

@matthieusieben matthieusieben left a comment

Choose a reason for hiding this comment

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

Look good

<LabelList className="-ml-1 flex-wrap">
{subjectStatus.tags.map((tag) => {
<LabelList className="-ml-1 flex-wrap gap-1">
{subjectStatus.tags.sort().map((tag) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Note that .sort() will sort the input array in place. I don't know if that's fine or if a copy should be made before sorting ?

Comment on lines 20 to 21
.map((t) => {
if (t.startsWith('lang:')) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You should probably trim here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the sort should be fine, we don't depend on the order anywhere

return list[0]
}

return `(${list.join(') OR (')})`
Copy link
Contributor

Choose a reason for hiding this comment

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

Are we mixing OR and && notations here ? Is that desired ?

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 could replace && with AND i guess since this is purely for display purposes.

export const SubjectTag = ({
tag,
...rest
}: { tag: string } & ComponentProps<typeof LabelChip>) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: For stricter types (and avoid any potential miss-use & conflicts), the props that will get ignored should not be allowed.

Suggested change
}: { tag: string } & ComponentProps<typeof LabelChip>) => {
}: { tag: string } & Omit<ComponentProps<typeof LabelChip>, 'tag' | 'children'>) => {

tagItemText: `text-gray-600 text-xs truncate cursor-default select-none`,
}

export const QueueFilterTags = () => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: This component is quite big. Could it be split into smaller components and communicate though props ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think it's quite reasonable in size and splitting into multiple components would just make unnecessarily more complex

@arcalinea arcalinea temporarily deployed to tag-filters - ozone-staging PR #266 January 8, 2025 17:06 — with Render Destroyed
@foysalit foysalit merged commit 63146b2 into main Jan 8, 2025
3 checks passed
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