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.

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

COL-564: [Frontend] Implement layout (header, sidebar, footer) #82

Merged
merged 1 commit into from
Nov 22, 2023

Conversation

snikidev
Copy link
Contributor

No description provided.

@snikidev snikidev self-assigned this Nov 10, 2023
Copy link
Collaborator

@mschristensen mschristensen left a comment

Choose a reason for hiding this comment

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

Haven't reviewed code yet, but couple of things running locally:

  • Clicking on a link in the left pane enables dark mode (Brave/chromium)
    Screenshot from 2023-11-13 10-08-53

  • The collapse/expand arrow on projects is the wrong way around

Screenshot from 2023-11-13 10-28-07

  • Linting is failing for me (possible it's picking up the eslint rules from the project root?):
pnpm run lint                                                                                                                                                   INT х  16m 37s  18.13.0 Node  10:24:43 

> livesync@0.1.0 lint /home/mike/code/models/examples/livesync
> next lint


./app/layout.tsx
4:1  Error: There should be at least one empty line between import groups  import/order
4:1  Error: `@radix-ui/themes` import should occur before type import of `next`  import/order
5:27  Error: Missing file extension "ts" for "@/components"  import/extensions

./components/AppLayout/AppLayout.tsx
2:1  Error: There should be at least one empty line between import groups  import/order
2:1  Error: `classnames` import should occur before import of `react`  import/order
3:1  Error: There should be at least one empty line between import groups  import/order
3:39  Error: Missing file extension "ts" for "@/components"  import/extensions
4:25  Error: Missing file extension "ts" for "../SideNav"  import/extensions
5:1  Error: `./AppLayout.module.css` import should occur before import of `../SideNav`  import/order

./components/AppLayout/index.ts
1:15  Error: Missing file extension "tsx" for "./AppLayout"  import/extensions

./components/Button/Button.tsx
3:1  Error: There should be at least one empty line between import groups  import/order
4:1  Error: `react` import should occur before import of `./Button.module.css`  import/order

./components/Button/index.ts
1:15  Error: Missing file extension "tsx" for "./Button"  import/extensions

./components/Header/Header.tsx
4:1  Error: `classnames` import should occur before import of `react`  import/order
5:1  Error: There should be at least one empty line between import groups  import/order
5:1  Error: `@radix-ui/themes` import should occur before import of `react`  import/order
6:43  Error: Missing file extension "ts" for "../icons"  import/extensions
7:25  Error: Missing file extension "ts" for "../SideNav"  import/extensions
8:1  Error: There should be no empty line within import group  import/order
8:1  Error: `../Button` import should occur before import of `../icons`  import/order
8:24  Error: Missing file extension "ts" for "../Button"  import/extensions
10:1  Error: `./Header.module.css` import should occur before import of `../icons`  import/order
11:1  Error: `../HowTo` import should occur before import of `../icons`  import/order
11:23  Error: Missing file extension "ts" for "../HowTo"  import/extensions

./components/Header/index.ts
1:15  Error: Missing file extension "tsx" for "./Header"  import/extensions

./components/HowTo/index.ts
1:15  Error: Missing file extension "tsx" for "./HowTo"  import/extensions

./components/PoweredByAbly/index.ts
1:15  Error: Missing file extension "tsx" for "./PoweredByAbly"  import/extensions

./components/SideNav/SideBarItem.tsx
4:1  Error: There should be at least one empty line between import groups  import/order
4:1  Error: `classnames` import should occur before import of `next/navigation`  import/order
6:1  Error: There should be at least one empty line between import groups  import/order
6:31  Error: Missing file extension "ts" for "../icons"  import/extensions
7:1  Error: `@radix-ui/react-accordion` import should occur before import of `next/navigation`  import/order

./components/SideNav/SideNav.tsx
3:1  Error: `classnames` import should occur before import of `next/navigation`  import/order
4:1  Error: There should be at least one empty line between import groups  import/order
4:1  Error: `@radix-ui/react-accordion` import should occur before import of `next/navigation`  import/order
5:29  Error: Missing file extension "tsx" for "./SideBarItem"  import/extensions
6:1  Error: There should be no empty line within import group  import/order
6:94  Error: Missing file extension "ts" for "../icons"  import/extensions
8:1  Error: There should be at least one empty line between import groups  import/order
8:1  Error: `./SideNav.module.css` import should occur before import of `../icons`  import/order
9:1  Error: There should be at least one empty line between import groups  import/order
9:1  Error: `next/link` import should occur before import of `next/navigation`  import/order
10:1  Error: There should be at least one empty line between import groups  import/order
10:1  Error: `../HowTo` import should occur before import of `../icons`  import/order
10:23  Error: Missing file extension "ts" for "../HowTo"  import/extensions
11:1  Error: There should be at least one empty line between import groups  import/order
11:1  Error: `@radix-ui/themes` import should occur before import of `next/navigation`  import/order
12:1  Error: `../Button` import should occur before import of `../icons`  import/order
12:24  Error: Missing file extension "ts" for "../Button"  import/extensions

./components/SideNav/index.ts
1:15  Error: Missing file extension "tsx" for "./SideNav"  import/extensions

./components/icons/index.ts
1:15  Error: Missing file extension "tsx" for "./Projects"  import/extensions
2:15  Error: Missing file extension "tsx" for "./Reporting"  import/extensions
3:15  Error: Missing file extension "tsx" for "./Dashboard"  import/extensions
4:15  Error: Missing file extension "tsx" for "./TeamMembers"  import/extensions
5:15  Error: Missing file extension "tsx" for "./ChevronUp"  import/extensions
6:15  Error: Missing file extension "tsx" for "./Menu"  import/extensions
7:15  Error: Missing file extension "tsx" for "./ExternalUrl"  import/extensions

./components/index.ts
1:15  Error: Missing file extension "ts" for "./SideNav"  import/extensions
2:15  Error: Missing file extension "ts" for "./Header"  import/extensions
3:15  Error: Missing file extension "ts" for "./Button"  import/extensions
4:15  Error: Missing file extension "ts" for "./AppLayout"  import/extensions
5:15  Error: Missing file extension "ts" for "./PoweredByAbly"  import/extensions

info  - Need to disable some ESLint rules? Learn more here: https://nextjs.org/docs/basic-features/eslint#disabling-rules
 ELIFECYCLE  Command failed with exit code 1
 ```
 
 

@dpiatek
Copy link

dpiatek commented Nov 13, 2023

when clicking on the menu, the text seems to flicker, like it is changing it's font or font size:
https://www.loom.com/share/550855a266e744c6b68f0e6328289286

@dpiatek
Copy link

dpiatek commented Nov 13, 2023

The initial code looks good to me - I'd suggest putting some of the colour, font and padding values into CSS variables.

@dpiatek
Copy link

dpiatek commented Nov 13, 2023

Should this be a link to ably.com ?

Screenshot 2023-11-13 at 11 49 16

@dpiatek
Copy link

dpiatek commented Nov 13, 2023

Missing icons here

Screenshot 2023-11-13 at 12 10 25

Copy link
Collaborator

@mschristensen mschristensen left a comment

Choose a reason for hiding this comment

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

Mostly looking great, however I think the breakpoints should kick in at a slightly larger width, as this is what it looks like on a Pixel 5:

Screenshot from 2023-11-20 10-56-50

Once fixed happy to approve, but would be good to please squash all these commits before we rebase into main. Thanks!

@@ -0,0 +1 @@
export * from './Button'
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor: missing newline

@@ -0,0 +1 @@
export * from './Header'
Copy link
Collaborator

Choose a reason for hiding this comment

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

minor: missing newline

@snikidev snikidev merged commit b75e2c6 into main Nov 22, 2023
6 checks passed
@snikidev snikidev deleted the col-564-layout branch November 22, 2023 16:10
@snikidev snikidev restored the col-564-layout branch November 23, 2023 14:19
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.

3 participants