Skip to content
This repository was archived by the owner on Apr 26, 2024. It is now read-only.

fix: side nav full width in mobile view.#1448

Merged
benhalverson merged 2 commits into
nodejs:mainfrom
MrJithil:SideNav_Mobile
Jun 23, 2021
Merged

fix: side nav full width in mobile view.#1448
benhalverson merged 2 commits into
nodejs:mainfrom
MrJithil:SideNav_Mobile

Conversation

@MrJithil

@MrJithil MrJithil commented Jun 19, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Description

This MR handles width issues caused in mobile devices. Previously the SideBar in mobile view was half in size. this MR will correct it.

Dark Mode Light Mode
image image

SideNavMob

Related Issues

#1446

https://staging.nodejs.dev/1448/privacy

@codecov-commenter

codecov-commenter commented Jun 19, 2021 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #1448 (c7e1a74) into main (993d0a6) will increase coverage by 0.19%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1448      +/-   ##
==========================================
+ Coverage   85.98%   86.18%   +0.19%     
==========================================
  Files          73       73              
  Lines         792      796       +4     
  Branches      216      220       +4     
==========================================
+ Hits          681      686       +5     
+ Misses        111      110       -1
Impacted Files Coverage Δ
src/components/AboutPageSideNavBar/index.tsx 100.00% <100.00%> (+9.09%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 993d0a6...c7e1a74. Read the comment docs.

@github-actions

github-actions Bot commented Jun 19, 2021

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/1448/

@MrJithil

MrJithil commented Jun 19, 2021

Copy link
Copy Markdown
Member Author

/preview

@github-actions

github-actions Bot commented Jun 19, 2021

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/1448/

@MrJithil MrJithil force-pushed the SideNav_Mobile branch 2 times, most recently from 691a6c9 to dab4855 Compare June 19, 2021 09:47
@MrJithil

MrJithil commented Jun 19, 2021

Copy link
Copy Markdown
Member Author

/preview

@MrJithil MrJithil force-pushed the SideNav_Mobile branch 8 times, most recently from e491dc5 to d4c4607 Compare June 19, 2021 19:12
@MrJithil MrJithil mentioned this pull request Jun 19, 2021
Merged
@MrJithil

MrJithil commented Jun 19, 2021

Copy link
Copy Markdown
Member Author

/preview

@github-actions

github-actions Bot commented Jun 19, 2021

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/1448/

@MrJithil MrJithil changed the title Fix: side nav full width in mobile view. WIP: Fix: side nav full width in mobile view. Jun 19, 2021
@MrJithil MrJithil force-pushed the SideNav_Mobile branch 2 times, most recently from 5fab9ea to 3b404c2 Compare June 19, 2021 19:54
@MrJithil MrJithil changed the title WIP: Fix: side nav full width in mobile view. Fix: side nav full width in mobile view. Jun 19, 2021
@MrJithil

MrJithil commented Jun 19, 2021

Copy link
Copy Markdown
Member Author

/preview

@github-actions

github-actions Bot commented Jun 19, 2021

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/1448/

@MrJithil MrJithil force-pushed the SideNav_Mobile branch 2 times, most recently from 25adfd4 to c62f313 Compare June 19, 2021 20:06
@MrJithil

MrJithil commented Jun 19, 2021

Copy link
Copy Markdown
Member Author

/preview

@github-actions

github-actions Bot commented Jun 19, 2021

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/1448/

@MrJithil MrJithil changed the title Fix: side nav full width in mobile view. [WIP]: Fix: side nav full width in mobile view. Jun 19, 2021
@MrJithil MrJithil changed the title [WIP]: Fix: side nav full width in mobile view. side nav full width in mobile view. Jun 19, 2021
@MrJithil MrJithil changed the title side nav full width in mobile view. fix: side nav full width in mobile view. Jun 19, 2021
@manishprivet manishprivet added the create-preview Generate preview on staging.nodejs.dev label Jun 22, 2021
@github-actions github-actions Bot removed the create-preview Generate preview on staging.nodejs.dev label Jun 22, 2021
@github-actions

github-actions Bot commented Jun 22, 2021

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/1448/

@manishprivet

manishprivet commented Jun 22, 2021

Copy link
Copy Markdown
Member

It seems like a consistent issue in Gatsby where unrelated CSS files are affecting pages across the site.
E.g., #1455 updated learn.scss for better positioning of the close button, and those are affecting this PR, where learn.scss is not imported anywhere on the page I think (Correct me if I'm wrong please).

image

@MrJithil Can you make a separate class name for the close button in about pages to fix this

@benhalverson benhalverson added the create-preview Generate preview on staging.nodejs.dev label Jun 22, 2021
@github-actions github-actions Bot removed the create-preview Generate preview on staging.nodejs.dev label Jun 22, 2021
@github-actions

github-actions Bot commented Jun 22, 2021

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/1448/

@MrJithil

MrJithil commented Jun 23, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

The code of learn.scss should be reverted to the original. And, we need to proceed with the changes in this or will solve the issues.

@manishprivet Please check the latest preview.

@MrJithil

MrJithil commented Jun 23, 2021

Copy link
Copy Markdown
Member Author

I have reverted the code and amend the changes here.

@MrJithil

MrJithil commented Jun 23, 2021

Copy link
Copy Markdown
Member Author

/preview

@github-actions

github-actions Bot commented Jun 23, 2021

Copy link
Copy Markdown

Please find a preview at: https://staging.nodejs.dev/1448/

@MrJithil

MrJithil commented Jun 23, 2021

Copy link
Copy Markdown
Member Author

SideNavMob

benhalverson
benhalverson approved these changes Jun 23, 2021
View reviewed changes

@benhalverson benhalverson left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

👍🏻

@benhalverson benhalverson merged commit b52f172 into nodejs:main Jun 23, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@benhalverson benhalverson benhalverson approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MrJithil @codecov-commenter @manishprivet @benhalverson