Skip to content

UI and Manager assigment #5

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

Open
wants to merge 15 commits into
base: master
Choose a base branch
from
Open

UI and Manager assigment #5

wants to merge 15 commits into from

Conversation

biel535
Copy link
Collaborator

@biel535 biel535 commented Jun 24, 2024

No description provided.

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

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
Member

Choose a reason for hiding this comment

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

+

Copy link
Member

@bykowskiolaf bykowskiolaf left a comment

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

package.json Outdated
Copy link
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",

@@ -17,7 +18,7 @@ const useHours = (date?: string) => {
refetch: hoursRefetch,
isLoading: hoursLoading,
isError: hoursError
} = useSuspenseQuery(hoursQueryOptions);
Copy link
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
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
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
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ć


if (!users) return <p>Brak użytkowników</p>;
if (meLoading) return <p>Loading...</p>;
Copy link
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
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 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.

4 participants