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

landing page overall fixes #35

Merged
merged 3 commits into from
Sep 30, 2024

Conversation

IgboPharaoh
Copy link
Collaborator

Copy link

vercel bot commented Sep 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
registry ✅ Ready (Inspect) Visit Preview 💬 Add feedback Sep 30, 2024 0:45am

@IgboPharaoh IgboPharaoh marked this pull request as ready for review September 19, 2024 17:09
Copy link
Member

@kouloumos kouloumos left a comment

Choose a reason for hiding this comment

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

Everything looks good overall! I've verified that all the issues have been resolved, except for the favicon fidelity. The favicon issue might require further discussion since the logo contains too many words to be displayed clearly in any setup.

A few other notes on this PR:

  • The navbar height is reduced and it's no longer sticky at the top. I actually prefer this change.
  • The hero section now starts as a single line, which is great. However, the second sentence should remain unbroken. If it wraps to a new line, the entire sentence should shift to the second line. @IgboPharaoh can we fix this nit?

One suggestion for future PRs: It would be helpful if each fix for a sub-issue was committed separately. This makes it easier to review specific changes, as the related code adjustments would be clear

@IgboPharaoh
Copy link
Collaborator Author

  • My mistake, the navbar has to be sticky, will add this change.
  • I don't know of a way to fix that nit. Either text follow the natural order and wrap inside it's container or they're broken and display on multiple lines.

@IgboPharaoh
Copy link
Collaborator Author

except for the favicon fidelity. The favicon issue might require further discussion since the logo contains too many words to be displayed clearly in any setup.

For the favicon, I think it's a discussion we need to have with the rest of the team especially Tobi.
I tried using figma to scale the current favicon to a 16 x 16 and 4 x 4 dimension but the transcripts text still makes it blurry at that size. We might want to change the favicons we use to be high fidelity icons only.

@kouloumos
Copy link
Member

My mistake, the navbar has to be sticky, will add this change.

In my previous comment I said that I liked the removal of the "stickiness". But I'm okay either way. Is that specified on the designs?

I don't know of a way to fix that nit. Either text follow the natural order and wrap inside it's container or they're broken and display on multiple lines.

Have you tried what I suggested here?

@IgboPharaoh
Copy link
Collaborator Author

My mistake, the navbar has to be sticky, will add this change.

In my previous comment I said that I liked the removal of the "stickiness". But I'm okay either way. Is that specified on the designs?

I don't know of a way to fix that nit. Either text follow the natural order and wrap inside it's container or they're broken and display on multiple lines.

Have you tried what I suggested here?

Addressed

Copy link
Collaborator

@0tuedon 0tuedon left a comment

Choose a reason for hiding this comment

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

Looks good, seems we have got the update of transcript favicon left. do we wait for tobi's update or just merge that in a later PR?

@Emmanuel-Develops Emmanuel-Develops linked an issue Sep 23, 2024 that may be closed by this pull request
7 tasks
@Emmanuel-Develops
Copy link
Collaborator

@IgboPharaoh
Please rest your branch to staging and cherry-pick the 3 commits added here.
Screenshot 2024-09-30 at 9 46 16 AM

@Emmanuel-Develops Emmanuel-Develops merged commit ef9eb67 into staging Sep 30, 2024
2 checks passed
Emmanuel-Develops pushed a commit to Emmanuel-Develops/registry that referenced this pull request Oct 8, 2024
* landing page overall fixes

* fix: make navbar sticky

* make subtext a standalone
Emmanuel-Develops added a commit that referenced this pull request Oct 11, 2024
* landing page overall fixes (#35)

* landing page overall fixes

* fix: make navbar sticky

* make subtext a standalone

* Transcript content page (#23)

* feat: base layout for explore

* feat: transcripts content page components

* fix: made scroll behaviour smooth

* feat: on scroll functionaity

* feat: mobile page and moving components from app to components

* fix: adding link on mobile

* refactor: created topics-data.json and chnaged grouping functions to match

* nit: modified contentlayer config

* feat: speakers grouping on explore

* feat: categories explore page

* nit: fix type error on exploreTranscript

* fix: breadcrumbs dynamic

* nit: removed .json from being tracked on git

* nit" changed package.json version

* nit: package updated to 1.3.0

* nit: changes linkName on categories

* fix: categories speakers in tags

* fix: removed .vscode and added to .gitignore

* nit: changed font sizes in layout and content grouping

* fix: moved all /transcripts to /categories

* design: added psuedo for select container

* fix: nit--fixes from PR #43 (#44)

---------

Co-authored-by: Solomon eze <[email protected]>
Co-authored-by: 0tuedon <[email protected]>
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.

Current Issues with the Landing Page
4 participants