Skip to content
This repository was archived by the owner on Mar 30, 2026. It is now read-only.

UI and Manager assigment#5

Open
biel535 wants to merge 15 commits into
masterfrom
feat/pdfPrint
Open

UI and Manager assigment#5
biel535 wants to merge 15 commits into
masterfrom
feat/pdfPrint

Conversation

@biel535

@biel535 biel535 commented Jun 24, 2024

Copy link
Copy Markdown
Contributor

No description provided.

const [currentMonth, setCurrentMonth] = useState<string>('2024-06');
const { me } = useMe();
const { userHours } = useUserHours(
selectedUser || '074221c8-1545-4e4b-a241-60addeaa0764'

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

co to za UUID, nie powinno być czegoś takiego, parametr opcjonalny

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+

@bykowskiolaf bykowskiolaf left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Generalnie dobry kod. Kilka małych poprawek:

  • Zmieniłbym strukture kodu, widziałem, że większość komponentów jest wrzucona do Users, fajnie to rozdzielić na te foldery gdzie jest używane
  • Niektóre z palca wpisane wartości, to na pewno nie przejdzie
  • React router jest po to żeby pomóc - nie przeszkadzac. Używaj więcej funkcjonalności od nich jeśli już jest w projekcie
  • To samo się tyczy do react query, super, że robiłeś hooki, ale możesz je wykorzystywać w routerze

Comment thread package.json

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Generalnie brakowało mi podczas odpalenia

    "@fullcalendar/daygrid": "^6.1.14",
    "@fullcalendar/interaction": "^6.1.14",
    "@fullcalendar/timegrid": "^6.1.14",

refetch: hoursRefetch,
isLoading: hoursLoading,
isError: hoursError
} = useSuspenseQuery(hoursQueryOptions);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Czemu nie SuspenseQuery? Router by się zajmował ładowaniem danych

const [currentMonth, setCurrentMonth] = useState<string>('2024-06');
const { me } = useMe();
const { userHours } = useUserHours(
selectedUser || '074221c8-1545-4e4b-a241-60addeaa0764'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

+

const calendarRef = useRef<FullCalendar | null>(null);

const getCurrentMonth = useCallback(() => {
if (!calendarRef.current) return '2024-06';

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Wartości z palca to raczej średnio

interface SelectUsersProps {
selectedUser: string | null;
setSelectedUser: (
userId: string | null | '074221c8-1545-4e4b-a241-60addeaa0764'

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Tutaj też lipa, nie może tak być

Comment thread src/pages/Users/index.tsx
const { me, meLoading, meError } = useMe();

if (!users) return <p>Brak użytkowników</p>;
if (meLoading) return <p>Loading...</p>;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do takich właśnie rzeczy jest react router i suspense query, nie trzeba sie martwic o loading

import assignUserToManager from '@/pages/Users/components/assignUserToManager';
import { createFileRoute } from '@tanstack/react-router';

export const Route = createFileRoute('/_dashboard/assignUserToManager')({

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Nie wiem czy potrzebna jest osobna strona do tego. Może dać to do edycji użytkownika?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants