Skip to content

Conversation

@wixzi
Copy link
Contributor

@wixzi wixzi commented May 6, 2023

Description

  • Added Navbar
  • Mobile menu
  • Mobile hamburger button and close
  • Mobile menu backdrop
  • Placeholder fields for wallet connect and user preview

@cloudflare-workers-and-pages
Copy link

cloudflare-workers-and-pages bot commented May 6, 2023

Deploying with  Cloudflare Pages  Cloudflare Pages

Latest commit: 2052cb9
Status: ✅  Deploy successful!
Preview URL: https://10174411.stackly-ui.pages.dev
Branch Preview URL: https://feature-stk-62-stackly-heade.stackly-ui.pages.dev

View logs

@berteotti
Copy link
Collaborator

Can you update .gitignore so there aren't build files on our repo?

@berteotti
Copy link
Collaborator

There is a mobile version on figma. Stackly header should adjust itself if is rendered on mobile or desktop.

1 similar comment
@berteotti
Copy link
Collaborator

There is a mobile version on figma. Stackly header should adjust itself if is rendered on mobile or desktop.

@wixzi
Copy link
Contributor Author

wixzi commented May 8, 2023

Can you update .gitignore so there aren't build files on our repo?

Its yarn cache and its required. Its not build files

@wixzi
Copy link
Contributor Author

wixzi commented May 8, 2023

There is a mobile version on figma. Stackly header should adjust itself if is rendered on mobile or desktop.

Will add the mobile classes. Mobile version I saw now.

@wixzi wixzi force-pushed the feature/STK-62-stackly-header branch 2 times, most recently from 9d2bb08 to 2beac8a Compare May 9, 2023 11:03
@wixzi wixzi marked this pull request as ready for review May 10, 2023 20:05
@Diogomartf
Copy link
Collaborator

Diogomartf commented May 11, 2023

Some things are missing on the navbar.

I let some prints just for reference:

image image

On mobile it doesn't need to ship with the correct buttons, but it has to have the structure to handle that positions.

Make sure to also use the correct icons for the hamburguer and close icon (they also have bg color).

You can merge with develop to get the tailwind theme.

@wixzi wixzi force-pushed the feature/STK-62-stackly-header branch from 0424999 to 30a74da Compare May 11, 2023 12:34
@wixzi
Copy link
Contributor Author

wixzi commented May 11, 2023

Some things are missing on the navbar.

I let some prints just for reference:

image image
On mobile it doesn't need to ship with the correct buttons, but it has to have the structure to handle that positions.

Make sure to also use the correct icons for the hamburguer and close icon (they also have bg color).

You can merge with develop to get the tailwind theme.

Thanks for checking

Copy link
Collaborator

@berteotti berteotti left a comment

Choose a reason for hiding this comment

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

Good job Velu, header looks nice. Just wondering if we should have the mobile logic all in the same component, using components like the placeholders and your stacks link in just 1 place.

Copy link
Collaborator

@Diogomartf Diogomartf left a comment

Choose a reason for hiding this comment

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

Overall good work Velu.

I've made a lot of components but mostly are about tailwindcss.
I think in the upcoming PRs will be better at tailwind, so I took more time on this one.

Also adding a small description of the work and screenshot in the PR description would be awesome.

Let's fix these minor issues and ship the navbar.

@wixzi wixzi force-pushed the feature/STK-62-stackly-header branch from 0ce0121 to 7862350 Compare May 12, 2023 16:23
@wixzi wixzi requested review from Diogomartf and berteotti May 12, 2023 16:36
@wixzi
Copy link
Contributor Author

wixzi commented May 12, 2023

Overall good work Velu.

I've made a lot of components but mostly are about tailwindcss. I think in the upcoming PRs will be better at tailwind, so I took more time on this one.

Also adding a small description of the work and screenshot in the PR description would be awesome.

Let's fix these minor issues and ship the navbar.

Thank you. It's been some time since I used Tailwind. Going forward this issue will not occur.

@wixzi wixzi force-pushed the feature/STK-62-stackly-header branch from 8b704d1 to 894c997 Compare May 12, 2023 16:45
@Diogomartf
Copy link
Collaborator

Just that small change and I think is ready ✔️

@berteotti
Copy link
Collaborator

Hey Velu, I advise you to download this vscode extension if you haven't already. It's pretty useful for this cases https://marketplace.visualstudio.com/items?itemName=bradlc.vscode-tailwindcss

@wixzi wixzi force-pushed the feature/STK-62-stackly-header branch from 894c997 to 62693a1 Compare May 15, 2023 17:13
@wixzi wixzi requested a review from Diogomartf May 15, 2023 17:14
Diogomartf
Diogomartf previously approved these changes May 15, 2023
Copy link
Collaborator

@berteotti berteotti left a comment

Choose a reason for hiding this comment

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

Let me know what you think about the constant

Copy link
Collaborator

@berteotti berteotti left a comment

Choose a reason for hiding this comment

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

Good job Velu!

@wixzi wixzi merged commit 686c7c7 into develop May 15, 2023
@wixzi wixzi deleted the feature/STK-62-stackly-header branch May 15, 2023 18:37
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