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

feature: Sorting & finding by username (ID) admin interface #1778

Merged
merged 9 commits into from Feb 21, 2023
Merged

feature: Sorting & finding by username (ID) admin interface #1778

merged 9 commits into from Feb 21, 2023

Conversation

owenduncansnobel
Copy link
Contributor

Feature changes for #1615

  • Auto-focuses on filter tab
  • Able to sort / filter by username (ID)
sort_filter.mov

@olliestanley olliestanley linked an issue Feb 21, 2023 that may be closed by this pull request
Sort/find users by username (ID) in admin interface #1615
Closed
AbdBarho
AbdBarho reviewed Feb 21, 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.

Thank you

website/src/components/DataTable/DataTable.tsx Outdated
@@ -35,7 +35,7 @@ import {
} from "@tanstack/react-table";
import { Filter } from "lucide-react";
import { useTranslation } from "next-i18next";
import { ChangeEvent, ReactNode, useState } from "react";
import { ChangeEvent, Dispatch, ReactNode, SetStateAction, useRef, useState } from "react";
Copy link
Collaborator

Choose a reason for hiding this comment

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

are all of these imports used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AbdBarho Not all of them are used, I'll remove them now. My first attempt used Dispatch and SetStateAction but I changed approaches at some point

website/src/components/DataTable/DataTable.tsx
@@ -198,13 +200,13 @@ const FilterModal = ({
value: string;
}) => {
const { isOpen, onOpen, onClose } = useDisclosure();

const initialFocusRef = useRef();
Copy link
Collaborator

Choose a reason for hiding this comment

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

what is the purpose of this ref?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@AbdBarho That ref is for auto-focusing on the text input when the filter button is clicked. That way you can start typing as soon as the parent div/button is clicked

notmd
notmd reviewed Feb 21, 2023
View reviewed changes
website/src/components/DataTable/DataTable.tsx Outdated
@@ -59,7 +61,7 @@ export type DataTableProps<T> = {
filterValues?: FilterItem[];
onNextClick?: () => void;
onPreviousClick?: () => void;
onFilterChange?: (items: FilterItem[]) => void;
onFilterChange?: (items: FilterItem[], filterByColumn?: string) => void;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why do you need filterByColumn here? Isn't id in FilterItem enough?

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 went and removed it

website/src/components/DataTable/DataTable.tsx Outdated Show resolved Hide resolved
website/src/components/UserTable.tsx Outdated
@@ -47,17 +50,23 @@ const columns: DataTableColumnDef<User>[] = [
export const UserTable = memo(function UserTable() {
const { pagination, resetCursor, toNextPage, toPreviousPage } = useCursorPagination();
const [filterValues, setFilterValues] = useState<FilterItem[]>([]);
const handleFilterValuesChange = (values: FilterItem[]) => {
const [filterOption, setFilterOption] = useState<string>("display_name");
Copy link
Collaborator

Choose a reason for hiding this comment

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

You dont need this state. The last FilterItem is the column you want, so just discard the rest values and keep the last one only

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 went and removed the state and used the last FilterItem

@github-actions
Copy link

❌ pre-commit failed.
Please run pre-commit run --all-files locally and commit the changes.
Find more information in the repository's CONTRIBUTING.md

@github-actions
Copy link

❌ pre-commit failed.
Please run pre-commit run --all-files locally and commit the changes.
Find more information in the repository's CONTRIBUTING.md

notmd
notmd requested changes Feb 21, 2023
View reviewed changes
Copy link
Collaborator

@notmd notmd left a comment

Choose a reason for hiding this comment

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

Thanks, look good now

website/src/components/UserTable.tsx Outdated Show resolved Hide resolved
website/src/components/DataTable/DataTable.tsx Outdated Show resolved Hide resolved
@notmd
Copy link
Collaborator

notmd commented Feb 21, 2023

Ah you need to update the handleFilterValuesChange, some thing like might work (please test it)

const handleFilterValuesChange = (values: FilterItem[]) => {
  const last = values.pop()
  if (last) {
    setFilterValues([last])
  }
  resetCursor();
}

notmd
notmd approved these changes Feb 21, 2023
View reviewed changes
Copy link
Collaborator

@notmd notmd left a comment

Choose a reason for hiding this comment

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

Thanks

@olliestanley olliestanley merged commit 96feac0 into LAION-AI:main Feb 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@notmd notmd notmd approved these changes

@fozziethebeat fozziethebeat Awaiting requested review from fozziethebeat

@AbdBarho AbdBarho Awaiting requested review from AbdBarho AbdBarho is a code owner

Assignees
No one assigned
Labels
website
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Sort/find users by username (ID) in admin interface
5 participants
@owenduncansnobel @notmd @AbdBarho @olliestanley @owen-duncan-snobel