UI: Header component #13

kpyszkowski · 2023-12-01T13:27:16Z

Ref: #2
~~Depends on: #12~~

This pull request introduces:

Logo component:
Header component:

Desktop

Tablet

Mobile
SelectWallet component redesign:

Additional changes:

Button component theme adjustments
Modal component theme adjustments

Note: Since this PR introduces changes widely used in the project eg. Modal or Button changes it's recommended to review it and merge primarily.

Rendered component in `App.tsx` entrypoint

Defined children components that composes Header's structure as separate components for improved readability and ease of future code maintanace. Left two TODOs: - it would be appropriate to utilise the fonts used in the project - it's unclear what is the BTC balance displayed in header from, for now it's mock value

Adjusted component's spacing accordingly to designs

Fixed spacings according to designs, fixed NavigationMenu container tag to be semantically correct - div to ul

Added theme tokens

Customized `Button` and `Alert` themes, modified `DotsLoadingIndicator` component.

Added control buttons: `AccountButton` and `NetworkButton`

Completed TODO to read token balance from user wallet data.

michalsmiarowski

Left some comments to look at before the merge, but overall good job 🔥

.env.example

src/components/Logo/Logo.tsx

michalsmiarowski · 2024-01-17T15:22:50Z

src/components/Logo/index.ts

@@ -0,0 +1 @@
+export { default as Logo } from "./Logo"


Non-blocking:

If you agree with the above and remove the default import, then we could just:

Suggested change

export { default as Logo } from "./Logo"

import * from "./Header"

This would be helpful in the case where we would want to export LogoProps - we would only had to add export to the interface and it would be exported automatically from src/components/Logo

I agree, but it breaks the overall idea of encapsulation and namespacing. The wildcard asterisk is in fact everything but that everything is undisclosed. The idea is to treat the index file as a "registry" of exported entities you can read through and have a clear view of the components' namespace at a first glance.
I added additional export declarations.

Ref commit: b9ed545

I think it was my mistake here, because we actually set the name of the default export to Logo. Still, I've mentioned that this could be better (although I admit, it's actually not) in case we would want to export LogoProps, but we don't need to export it right now.

I would propose to not export types if it's not necessary, and we don't use HeaderProps and LogoProps outside of the component. What do you think about that?

If you agree then we should remove the changes from b9ed545.

Reverted

Ref commit: 3b161e4

As a side note: exporting props type definition might be useful. In my opinion better approach would be to always export it instead of exporting on demand - just for consistency. Nevertheless the rules and guidelines of structuring the project are not formally standardized in any way so currently it's a matter of preference. I don't stickle against reverting changes at the moment. If we were to define such standards I would opt for my point of view, just for consistency as I said.

src/components/Modal/SelectWalletModal/components/WalletConnectionModalBase.tsx

src/theme/index.ts

michalsmiarowski · 2024-01-22T09:00:28Z

src/theme/index.ts

@@ -1,5 +1,4 @@
 import { extendTheme } from "@chakra-ui/react"
-import { mode } from "@chakra-ui/theme-tools"
 import { defaultTheme } from "@threshold-network/components"


Non-blocking:

This should be addressedi na separate PR, but ideally we should not use @threshold-network/components in this repo as teh designs are not related to threshold dashboard at all 😉 Just pointing that out to not forget about it and I want to know if you agree.

Correct. I agree. We just have to consider whether it's worth the effort if we want to start a completely new project anyway. I leave the decision up to you 🙇🏼

Probably not. We can always return to this issue if it turn out that we will continue from this codebase.

michalsmiarowski · 2024-01-22T09:02:03Z

src/theme/Alert.ts

+    color: "#66FFB6",
+    bg: "#1E1E1E",


Non-blocking but something to discuss in the future:

Ideally, we should put all the colors in the theme and name it. But I think this should be also specified in the designs.

Definitely. But I have no idea how to name/structure it. It's difficult to use incremental pattern here. Do you have any ideas?

src/theme/Alert.ts

Added comment to explain the value

`asLink` to `asLinkToHomepage`

Refactored component definitions for `Header` and `Logo` components.

`border` => `whiteAlpha`

kpyszkowski

Addressed comments.

.env.example

kpyszkowski · 2024-01-22T12:10:05Z

src/components/Header/Header.children.tsx

+export function NavigationMenu(props: NavigationMenuProps) {
+  const { items } = props
+  return (
+    <Flex as={List} alignSelf={"stretch"} ml={146 - 20} mr={"auto"}>


It should be 142 - 20. Fixed.
As you can see on screenshot I added horizontal padding for NavigationItems to make it more accessible - easier to click by enlarging the area. To keep it pixel perfect the margin between Logo and NavigationMenu must be reduced by the item padding so spacing is now 122px of menu margin + 20px of item padding which results as 142px as it states in designs.

I've added comment to explain the math behind the operation.
Ref commit: 3c9f8aa

src/components/Header/Header.children.tsx

kpyszkowski · 2024-01-22T12:19:37Z

src/components/Header/Header.tsx

+import { Logo } from "../Logo"
+import { NavigationMenu, UserPanel } from "./Header.children"
+
+// TODO: Load new fonts


I think it's reasonable to do it in separate PR to prevent blocking this PR.
I wasn't able to make it - not sure how Chakra theming works in terms of font setting.
https://discord.com/channels/893441201628405760/1181551917332185148/1181634992409944204

src/components/Header/Header.tsx

src/components/Modal/SelectWalletModal/components/WalletConnectionModalBase.tsx

src/theme/Alert.ts

kpyszkowski · 2024-01-22T13:38:23Z

src/theme/Alert.ts

+    color: "#66FFB6",
+    bg: "#1E1E1E",


Definitely. But I have no idea how to name/structure it. It's difficult to use incremental pattern here. Do you have any ideas?

src/theme/index.ts

kpyszkowski · 2024-01-22T14:52:06Z

src/theme/index.ts

@@ -1,5 +1,4 @@
 import { extendTheme } from "@chakra-ui/react"
-import { mode } from "@chakra-ui/theme-tools"
 import { defaultTheme } from "@threshold-network/components"


Correct. I agree. We just have to consider whether it's worth the effort if we want to start a completely new project anyway. I leave the decision up to you 🙇🏼

Implemented responsive Navigation menu with toggleable drawer menu, backdrop and animations. Maximum width of the drawer is `sm` breakpoint. The chain indicator is hidden for mobile, it appears when `md` breakpoint is passed. Added alternative smaller variant of logo size. Also excluded logo from blurred backdrops so now it goes on top remaining visible. It's a catchy detail reminding user what application is used.

After adding responsive menu the .children file became lengthy. I decided to restructure the namespace dividing components that form the `Header` component into separate files.

michalsmiarowski

Left some comments to look at before the merge

michalsmiarowski · 2024-01-24T14:51:41Z

src/components/Header/NavigationMenu.tsx

+    stroke-width={1}
+    stroke-linecap="round"
+    stroke-linejoin="round"


Should be:

Suggested change

stroke-width={1}

stroke-linecap="round"

stroke-linejoin="round"

strokeWidth={1}

strokeLinecap="round"

strokeLinejoin="round"

The actual implementation produces warnings in the console.

Ref commit: 7bb9859

michalsmiarowski · 2024-01-24T14:55:01Z

src/theme/index.ts

@@ -1,5 +1,4 @@
 import { extendTheme } from "@chakra-ui/react"
-import { mode } from "@chakra-ui/theme-tools"
 import { defaultTheme } from "@threshold-network/components"


Probably not. We can always return to this issue if it turn out that we will continue from this codebase.

This reverts commit b9ed545.

kpyszkowski

Addressed comments

kpyszkowski · 2024-01-24T16:06:51Z

src/components/Header/NavigationMenu.tsx

+    stroke-width={1}
+    stroke-linecap="round"
+    stroke-linejoin="round"


Ref commit: 7bb9859

kpyszkowski · 2024-01-24T16:07:56Z

src/components/Header/Header.tsx

+import { Logo } from "../Logo"
+import { NavigationMenu, UserPanel } from "./Header.children"
+
+// TODO: Load new fonts


Already applied font changes. Ref PR: #30

kpyszkowski · 2024-01-24T16:21:15Z

src/components/Logo/index.ts

@@ -0,0 +1 @@
+export { default as Logo } from "./Logo"


Reverted

Ref commit: 3b161e4

As a side note: exporting props type definition might be useful. In my opinion better approach would be to always export it instead of exporting on demand - just for consistency. Nevertheless the rules and guidelines of structuring the project are not formally standardized in any way so currently it's a matter of preference. I don't stickle against reverting changes at the moment. If we were to define such standards I would opt for my point of view, just for consistency as I said.

michalsmiarowski

LGTM 🔥

kpyszkowski added 4 commits November 30, 2023 19:44

Created base for Header component

74ed827

Rendered component in `App.tsx` entrypoint

Implement Logo component

9571039

Adjust Header's UserPanel spacing

e7d6465

Adjusted component's spacing accordingly to designs

kpyszkowski mentioned this pull request Dec 1, 2023

MVP: Initial landing page #2

Closed

kpyszkowski requested a review from michalsmiarowski December 5, 2023 14:22

kpyszkowski added 2 commits December 5, 2023 16:10

Merge branch 'main-cleanup' into ui/header-component

0e687e1

Merge branch 'main-cleanup' into ui/header-component

474a5f4

kpyszkowski added the 💎 components label Dec 21, 2023

kpyszkowski added 5 commits December 28, 2023 17:43

Remove redundant export directives

42aa0c1

Add Header fixed width container

533e129

Fixed spacings according to designs, fixed NavigationMenu container tag to be semantically correct - div to ul

Adjust Button component theme

4de9b63

Added theme tokens

Restyle SelectWalletModal component

c105104

Customized `Button` and `Alert` themes, modified `DotsLoadingIndicator` component.

Adjust Header's UserPanel component

a69046a

Added control buttons: `AccountButton` and `NetworkButton`

kpyszkowski mentioned this pull request Jan 8, 2024

UI: Make Deposit step #21

Merged

Merge branch 'main' into ui/header-component

639785d

michalsmiarowski assigned kpyszkowski Jan 17, 2024

Add Header's UserPanel menu

67b2549

Completed TODO to read token balance from user wallet data.

michalsmiarowski requested changes Jan 22, 2024

View reviewed changes

kpyszkowski added 7 commits January 22, 2024 12:37

Remove redundant changes in .env.example file

f113c2e

Finetune margin value for NavigationMenu

3c9f8aa

Added comment to explain the value

Rename Logo prop

838dcc3

`asLink` to `asLinkToHomepage`

Refactor component definitions

3a23252

Refactored component definitions for `Header` and `Logo` components.

Exported props definitions

b9ed545

Remove redundant boolean casting

7329cef

Update Chakra theme config

5252f89

`border` => `whiteAlpha`

kpyszkowski commented Jan 22, 2024

View reviewed changes

kpyszkowski requested a review from michalsmiarowski January 22, 2024 15:01

kpyszkowski added 2 commits January 23, 2024 19:38

Restructure Header component namespace

d2ce507

After adding responsive menu the .children file became lengthy. I decided to restructure the namespace dividing components that form the `Header` component into separate files.

michalsmiarowski requested changes Jan 24, 2024

View reviewed changes

kpyszkowski added 2 commits January 24, 2024 17:05

Change casing of SVG properties to resolve warning

7bb9859

Revert "Exported props definitions"

3b161e4

This reverts commit b9ed545.

kpyszkowski commented Jan 24, 2024

View reviewed changes

michalsmiarowski mentioned this pull request Jan 24, 2024

UI: PageLayout component #15

Merged

michalsmiarowski self-requested a review January 25, 2024 14:51

michalsmiarowski approved these changes Jan 25, 2024

View reviewed changes

Merge branch 'main' into ui/header-component

764b0c9

michalsmiarowski merged commit 3db2d75 into main Jan 25, 2024
2 checks passed

kpyszkowski mentioned this pull request Feb 2, 2024

MVP: Connect wallet action #3

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

UI: Header component #13

UI: Header component #13

kpyszkowski commented Dec 1, 2023 •

edited

Loading

michalsmiarowski left a comment

michalsmiarowski Jan 17, 2024

kpyszkowski Jan 22, 2024 •

edited

Loading

michalsmiarowski Jan 24, 2024

kpyszkowski Jan 24, 2024

michalsmiarowski Jan 22, 2024

kpyszkowski Jan 22, 2024

michalsmiarowski Jan 24, 2024

michalsmiarowski Jan 22, 2024

kpyszkowski Jan 22, 2024

kpyszkowski left a comment

kpyszkowski Jan 22, 2024 •

edited

Loading

kpyszkowski Jan 22, 2024

kpyszkowski Jan 22, 2024

kpyszkowski Jan 22, 2024

michalsmiarowski left a comment •

edited

Loading

michalsmiarowski Jan 24, 2024

kpyszkowski Jan 24, 2024

michalsmiarowski Jan 24, 2024

kpyszkowski left a comment

kpyszkowski Jan 24, 2024

kpyszkowski Jan 24, 2024

kpyszkowski Jan 24, 2024

michalsmiarowski left a comment

	export { default as Logo } from "./Logo"
	import * from "./Header"

UI: Header component #13

UI: Header component #13

Conversation

kpyszkowski commented Dec 1, 2023 • edited Loading

michalsmiarowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kpyszkowski Jan 22, 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

Choose a reason for hiding this comment

kpyszkowski left a comment

Choose a reason for hiding this comment

kpyszkowski Jan 22, 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

michalsmiarowski left a comment • 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

kpyszkowski 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

michalsmiarowski left a comment

Choose a reason for hiding this comment

kpyszkowski commented Dec 1, 2023 •

edited

Loading

kpyszkowski Jan 22, 2024 •

edited

Loading

kpyszkowski Jan 22, 2024 •

edited

Loading

michalsmiarowski left a comment •

edited

Loading