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.

Sign up for GitHub

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

Jump to bottom

Web tidy sweep #583

Merged
merged 17 commits into from
Jan 10, 2023
Merged

Web tidy sweep #583

merged 17 commits into from
Jan 10, 2023

Conversation

fozziethebeat
Copy link
Collaborator

@fozziethebeat fozziethebeat commented Jan 10, 2023 •
edited
Loading

Closes #369
Closes #310
Closes #371
Closes #587

Another sweep of tidying fixes:

  • Cleanup more eslint warnings
  • Update the FlaggableElement to call off() when reporting is done
  • Update the MessageView to be forwardRef which is required by PopoverAnchor
  • Update the reject task handler to use both prisma and Prisma
  • Ensure that FlaggableElement has valid labels in every call site.

@fozziethebeat fozziethebeat marked this pull request as ready for review January 10, 2023 05:22
AbdBarho
AbdBarho reviewed Jan 10, 2023
View reviewed changes
Copy link
Collaborator

@AbdBarho AbdBarho 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 will have a lot of conflicts with #575

should we merge that one first or this one?

website/src/components/Messages/MessageTable.tsx Outdated
@@ -1,11 +1,12 @@
import { Stack, StackDivider } from "@chakra-ui/react";
import { MessageTableEntry } from "src/components/Messages/MessageTableEntry";

export function MessageTable({ messages }) {
export function MessageTable({ messages, valid_labels }) {
console.log(messages);
Copy link
Collaborator

Choose a reason for hiding this comment

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

oh oh

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Removed!

@fozziethebeat
Copy link
Collaborator Author

I say merge your PR first then I'll handle the conflicts here. They might not actually be that bad.

k-nearest-neighbor
k-nearest-neighbor approved these changes Jan 10, 2023
View reviewed changes
Copy link
Collaborator

@k-nearest-neighbor k-nearest-neighbor left a comment •
edited
Loading

Choose a reason for hiding this comment

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

LGTM

@fozziethebeat
Copy link
Collaborator Author

I'll merge this now so that any new changes don't have to deal with potential conflicts.

@fozziethebeat fozziethebeat merged commit 600f7f7 into main Jan 10, 2023
@fozziethebeat fozziethebeat deleted the web-tidy-sweep branch January 10, 2023 07:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AbdBarho AbdBarho AbdBarho left review comments

@k-nearest-neighbor k-nearest-neighbor k-nearest-neighbor approved these changes

Assignees
No one assigned
Labels
website
Projects
None yet
Milestone
No milestone
4 participants
@fozziethebeat @AbdBarho @k-nearest-neighbor @yk