Feature: #22 TopMenu #60

sepehrsohrabii · 2024-05-07T16:41:07Z

No description provided.

sepehrsohrabii · 2024-05-12T11:17:55Z

src/components/TopMenu/TopMenu.spec.tsx

This file will be changed in the test PRs, So it doesn't need to be reviewed.

Please move this component to the shared/components/TopMenu

src/components/Dropdown/DropdownItem/DropdownItem.spec.tsx

src/components/Dropdown/DropdownHeader/DropdownHeader.spec.tsx

src/components/Dropdown/DropdownDivider/DropdownDivider.spec.tsx

sepehrsohrabii · 2024-05-12T11:19:54Z

src/components/TopMenu/TopMenuLinks/TopMenuLinks.spec.tsx

This file will be changed in the test PRs, So it doesn't need to be reviewed.

sepehrsohrabii · 2024-05-12T11:20:03Z

src/components/TopMenu/TopMenuUserMenu/TopMenuUserMenu.spec.tsx

This file will be changed in the test PRs, So it doesn't need to be reviewed.

Property 'pic' does not exist on type '{ address: string; } & { name?: string | null | undefined; email?: string | null | undefined; image?: string | null | undefined; }'.

And what kind of name is TopMenuUserMenu? two times menu?!

This test failed. 4 fails! Also, you did it wrongly. You should make a mock instance and then run mockReturnValue or reset or clear it. You can run: useSession.mockReturnValue({ data: null }); directly because you didn't mock it!

Thank you, All of the above-mentioned items have been done except the item regarding tests which I will solve in the test issue.

sepehrsohrabii · 2024-05-12T11:22:44Z

src/styles/fonts.test.ts

Here I changed the Roboto font to Roboto_Flex font because the Nextjs document recommended using variable fonts because of performance. So I replaced Roboto with Roboto_Flex which is a variable font in the main fonts file and continuously in its test file

sepehrsohrabii · 2024-05-12T11:24:12Z

src/styles/fonts.ts

Here I changed the Roboto font to Roboto_Flex font because the Nextjs document recommended using variable fonts because of performance. So I replaced Roboto with Roboto_Flex which is a variable font in the main fonts file

sepehrsohrabii · 2024-05-12T11:26:04Z

src/utils/i18n/client.js

I tried to fix the i18next problem. So, these are not complete please don't review them. I should work on it in the future

sepehrsohrabii · 2024-05-12T11:26:18Z

src/utils/i18n/i18n.js

I tried to fix the i18next problem. So, these are not complete please don't review them. I should work on it in the future.

sepehrsohrabii · 2024-05-12T11:27:51Z

src/utils/i18n/index.js

These changes are because of my vscode so I think this file doesn't need review.

haghighi251 · 2024-05-12T20:22:33Z

src/components/Navbar/NavbarToggle/NavbarToggle.tsx

@@ -23,7 +23,7 @@ const NavbarToggle = forwardRef<any, NavbarToggleProps>(
                id={id}
                className={className}
                data-testid={testId}
-                onClick={onClick}
+                // onClick={onClick}


Why did you do this?

Because, The Navbar Toggle flowbite component has a hidden onClick function, and there is no way to override it. So adding onClick will deactivate the default onClick. Finally, it will not work as a toggle in the navbar.

haghighi251

There are some problems here:
When we switch to persian and then english, some data are still in Persian and vice versa.
In the persian the top menu home link is stuck to the next item.
When we are switching from a language to another one the login/register button is still on the prior language.

haghighi251 · 2024-05-12T20:30:37Z

src/components/TopMenu/TopMenuUserMenu/TopMenuUserMenu.spec.tsx

+
+describe('TopMenuUserMenu', () => {
+    afterEach(() => {
+        jest.clearAllMocks(); // Clear mock calls after each test


You have to make an instance and then reset it here:
useSessionMock.clearMock()

haghighi251 · 2024-05-12T20:31:23Z

src/components/TopMenu/TopMenuUserMenu/TopMenuUserMenu.spec.tsx

+
+// Mock the useSession hook
+jest.mock('next-auth/react', () => ({
+    useSession: jest.fn(),


const useSessionMock = jest.fn();
useSession: useSessionMock
....
Do the same thing also for login and logout methods.

haghighi251 · 2024-05-12T20:32:27Z

src/components/TopMenu/TopMenuUserMenu/TopMenuUserMenu.spec.tsx

+
+describe('TopMenuUserMenu', () => {
+    afterEach(() => {
+        jest.clearAllMocks(); // Clear mock calls after each test


This comment is not clearing anything here.

haghighi251 · 2024-05-12T20:33:10Z

src/components/TopMenu/TopMenuUserMenu/TopMenuUserMenu.spec.tsx

+    });
+
+    it('renders login button when user is not authenticated', () => {
+        useSession.mockReturnValue({ data: null }); // Mock unauthenticated session


This is not a real mock and is running the real implementation of the method because you didn't mock it so this reset is not working here.

haghighi251 · 2024-05-12T20:34:14Z

src/components/TopMenu/TopMenuUserMenu/TopMenuUserMenu.spec.tsx

+
+    it('renders user menu when user is authenticated', () => {
+        const mockUser = {
+            name: 'Bonnie Green',


mockUser should be public and then it could be used also in other tests.

haghighi251 · 2024-05-30T19:05:25Z

src/shared/components/TopMenu/TopUserMenu/TopUserMenu.tsx

+                label={
+                    <Avatar
+                        alt={`${session.user.name} profile picture`}
+                        placeholderInitials={session.user.name


You shouldn't do this kind of things in the JSX.

haghighi251 · 2024-05-30T19:06:33Z

src/shared/components/TopMenu/TopUserMenu/TopUserMenu.spec.tsx

+        const { getByText } = render(<TopUserMenu />);
+        fireEvent.click(getByText('Sign Out'));
+        await waitFor(() => {
+            expect(signOut).toHaveBeenCalled();


How many times? With which data?!

haghighi251 · 2024-05-30T19:06:53Z

src/shared/components/TopMenu/TopUserMenu/TopUserMenu.spec.tsx

+        useSession.mockReturnValue({ data: null }); // Mock unauthenticated session
+        const { getByText } = render(<TopUserMenu />);
+        fireEvent.click(getByText('Login / Register'));
+        expect(signIn).toHaveBeenCalled();


How many times? With which data?!

haghighi251 · 2024-05-30T19:07:21Z

src/shared/components/TopMenu/TopUserMenu/TopUserMenu.spec.tsx

+            email: '[email protected]',
+            pic: 'https://flowbite.com/docs/images/people/profile-picture-5.jpg',
+        };
+        useSession.mockReturnValue({ data: { user: mockUser } }); // Mock authenticated session


Remove ChatGPT comments.

haghighi251 · 2024-05-30T19:07:49Z

src/shared/components/TopMenu/TopUserMenu/TopUserMenu.spec.tsx

+    });
+
+    it('renders user menu when user is authenticated', () => {
+        const mockUser = {


Declare the mock user as a public user when use are using it in some test with the same data.

sepehrsohrabii changed the base branch from production to development May 7, 2024 16:41

sepehrsohrabii force-pushed the feature/22-top-menu branch from 7014fe2 to c56ba3b Compare May 12, 2024 11:15

sepehrsohrabii commented May 12, 2024

View reviewed changes

src/components/Dropdown/DropdownItem/DropdownItem.spec.tsx Outdated Show resolved Hide resolved

sepehrsohrabii commented May 12, 2024

View reviewed changes

src/components/Dropdown/DropdownHeader/DropdownHeader.spec.tsx Outdated Show resolved Hide resolved

sepehrsohrabii commented May 12, 2024

View reviewed changes

src/components/Dropdown/DropdownDivider/DropdownDivider.spec.tsx Outdated Show resolved Hide resolved

sepehrsohrabii commented May 12, 2024

View reviewed changes

src/utils/i18n/index.js Outdated

Copy link

Collaborator Author

sepehrsohrabii May 12, 2024

Choose a reason for hiding this comment

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

These changes are because of my vscode so I think this file doesn't need review.

sepehrsohrabii requested a review from haghighi251 May 12, 2024 11:28

sepehrsohrabii self-assigned this May 12, 2024

haghighi251 reviewed May 12, 2024

View reviewed changes

sepehrsohrabii requested a review from haghighi251 May 15, 2024 14:26

sepehrsohrabii force-pushed the feature/22-top-menu branch from 66a5488 to 99326ff Compare May 26, 2024 09:54

sepehrsohrabii force-pushed the development branch from 9024fa9 to cb1491c Compare May 27, 2024 15:58

sepehrsohrabii added 10 commits May 29, 2024 18:23

fix: fixed the layout type path

44a4b39

feat: completed remainings from dropdown

ca6a113

feat: added links to topmenu

c39892d

feat: navbar links translation completed

9f4f987

feat: fixed client side errors

edc7c35

feat: working on i18n library configuration

62998e1

feat: added forwardRef to top menu

f81146c

refactor: renamed the TopMenuUserMenu

eddd64c

fix: removed onclick comment from navbar toggle

aec09a8

fix: removed unnecessary comments

076779d

sepehrsohrabii force-pushed the feature/22-top-menu branch from 99326ff to 076779d Compare May 29, 2024 14:54

haghighi251 reviewed May 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature: #22 TopMenu #60

Feature: #22 TopMenu #60

sepehrsohrabii commented May 7, 2024

sepehrsohrabii May 12, 2024

haghighi251 May 12, 2024

sepehrsohrabii May 15, 2024

sepehrsohrabii May 12, 2024

sepehrsohrabii May 12, 2024

haghighi251 May 12, 2024

haghighi251 May 12, 2024

haghighi251 May 12, 2024

sepehrsohrabii May 15, 2024

sepehrsohrabii May 12, 2024 •

edited

Loading

sepehrsohrabii May 12, 2024

sepehrsohrabii May 12, 2024

sepehrsohrabii May 12, 2024

sepehrsohrabii May 12, 2024

haghighi251 May 12, 2024

sepehrsohrabii May 15, 2024

haghighi251 left a comment

haghighi251 May 12, 2024

haghighi251 May 12, 2024

haghighi251 May 12, 2024

haghighi251 May 12, 2024

haghighi251 May 12, 2024

haghighi251 May 30, 2024

haghighi251 May 30, 2024

haghighi251 May 30, 2024

haghighi251 May 30, 2024

haghighi251 May 30, 2024

Feature: #22 TopMenu #60

Are you sure you want to change the base?

Feature: #22 TopMenu #60

Conversation

sepehrsohrabii commented May 7, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sepehrsohrabii May 12, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

haghighi251 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sepehrsohrabii May 12, 2024 •

edited

Loading