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.

Sign up for GitHub

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

Jump to bottom

Added navigation for mobile devices #2198

Merged
merged 8 commits into from
Dec 5, 2023
Merged

Added navigation for mobile devices #2198

merged 8 commits into from
Dec 5, 2023

Conversation

MAXASSASSINS
Copy link
Contributor

@MAXASSASSINS MAXASSASSINS commented Nov 30, 2023 •
edited by Anmol-Baranwal
Loading

Fixes Issue

closes #2196

Changes proposed

  • Created a new component handling navigation in mobile devices.
  • Added some css in general layout.
  • Fixed the width of the linkshub description on home page.

Screenshots

Screenshot 2023-12-01 at 1 42 53 AM Screenshot 2023-12-01 at 1 44 41 AM

Note to reviewers

@vercel Vercel
Copy link

vercel bot commented Nov 30, 2023 •
edited
Loading

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

Name Status Preview Comments Updated (UTC)
linkshub ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 1, 2023 6:12pm

github-actions[bot]
github-actions bot reviewed Nov 30, 2023
View reviewed changes
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Thank you, MAXASSASSINS, for creating this pull request and contributing to LinksHub! 💗

The maintainers will review this Pull Request and provide feedback as soon as possible! 😇
We appreciate your patience and contribution, Keep up the great work! 😀

CBID2
CBID2 approved these changes Dec 1, 2023
View reviewed changes
Copy link
Collaborator

@CBID2 CBID2 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 to me! @Anmol-Baranwal, @rupali-codes, check this out! 😁

aftabrehan
aftabrehan requested changes Dec 1, 2023
View reviewed changes
Copy link
Collaborator

@aftabrehan aftabrehan left a comment

Choose a reason for hiding this comment

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

Excellent PR, @MAXASSASSINS 💪 🚀 💯

It just needs minor adjustments and then we're good to go 🚀

components/MobileBottomNav/MobileBottomNav.tsx Outdated Show resolved Hide resolved
layouts/GeneralLayout.tsx Outdated Show resolved Hide resolved
components/MobileBottomNav/MobileBottomNav.tsx Show resolved Hide resolved
aftabrehan
aftabrehan approved these changes Dec 1, 2023
View reviewed changes
Copy link
Collaborator

@aftabrehan aftabrehan 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 to me! 👍

Just a minor fix required.

components/MobileBottomNav/MobileBottomNav.tsx Outdated Show resolved Hide resolved
components/MobileBottomNav/MobileBottomNav.tsx Outdated Show resolved Hide resolved
@Anmol-Baranwal Anmol-Baranwal linked an issue Dec 1, 2023 that may be closed by this pull request
feat: add new design of nav menu in mobile devices #2196
Closed
Anmol-Baranwal
Anmol-Baranwal requested changes Dec 1, 2023
View reviewed changes
Copy link
Collaborator

@Anmol-Baranwal Anmol-Baranwal left a comment

Choose a reason for hiding this comment

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

@MAXASSASSINS

Please make the following changes; they might resolve deployment issues:

  1. Change commonCls from let to const.

Since the codebase follows proper standards, it gives an error regarding this.

  1. I don't think QuestionMarkIcon, useEffect, useState are used in MobileBottomNav. Please remove them if they are extra code.

Anmol-Baranwal
Anmol-Baranwal approved these changes Dec 1, 2023
View reviewed changes
Copy link
Collaborator

@Anmol-Baranwal Anmol-Baranwal left a comment

Choose a reason for hiding this comment

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

@MAXASSASSINS

🚀 Looks good to me.

Thanks! It is deployed.

aftabrehan
aftabrehan approved these changes Dec 3, 2023
View reviewed changes
Copy link
Collaborator

@aftabrehan aftabrehan 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 to me! 👍

@aftabrehan aftabrehan added the status: ready-to-merge Approved & its ready-to-merge label Dec 3, 2023
@Anmol-Baranwal
Copy link
Collaborator

@rupali-codes
Review and merge this :)

rupali-codes
rupali-codes approved these changes Dec 5, 2023
View reviewed changes
Copy link
Owner

@rupali-codes rupali-codes left a comment

Choose a reason for hiding this comment

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

looks perfect

@rupali-codes rupali-codes merged commit 653dcfb into rupali-codes:dev Dec 5, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@github-actions GitHub Actions GitHub Actions left review comments

@Anmol-Baranwal Anmol-Baranwal Anmol-Baranwal approved these changes

@rupali-codes rupali-codes rupali-codes approved these changes

@aftabrehan aftabrehan aftabrehan approved these changes

@CBID2 CBID2 CBID2 approved these changes

Assignees

@MAXASSASSINS MAXASSASSINS

Labels
status: ready-to-merge Approved & its ready-to-merge
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

feat: add new design of nav menu in mobile devices
5 participants
@MAXASSASSINS @Anmol-Baranwal @rupali-codes @aftabrehan @CBID2