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

Fix: Close button position on learn page#1455

Merged
benhalverson merged 3 commits into
nodejs:mainfrom
manishprivet:learn-mobile-navbar-fix
Jun 22, 2021
Merged

Fix: Close button position on learn page#1455
benhalverson merged 3 commits into
nodejs:mainfrom
manishprivet:learn-mobile-navbar-fix

Conversation

@manishprivet

@manishprivet manishprivet commented Jun 20, 2021

Copy link
Copy Markdown
Member

Description

Fixes close button position in learn page on mobile

Related Issues

Fixes #1453

@codecov-commenter

codecov-commenter commented Jun 20, 2021 •
edited
Loading

Copy link
Copy Markdown

Codecov Report

Merging #1455 (80fca95) into main (7dcb78f) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1455   +/-   ##
=======================================
  Coverage   85.98%   85.98%           
=======================================
  Files          73       73           
  Lines         792      792           
  Branches      216      216           
=======================================
  Hits          681      681           
  Misses        111      111

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 7dcb78f...80fca95. Read the comment docs.

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

github-actions Bot commented Jun 20, 2021

Copy link
Copy Markdown

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

rodion-arr
rodion-arr reviewed Jun 20, 2021
View reviewed changes

@rodion-arr rodion-arr left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks cut on 414×719px screen
image

@manishprivet

manishprivet commented Jun 21, 2021

Copy link
Copy Markdown
Member Author

@rodion-arr Is it possible that the site is intentionally zoomed in on your screen?
As I can't reproduce the issue here

image

@rodion-arr

rodion-arr commented Jun 21, 2021

Copy link
Copy Markdown
Contributor

Nope, I'm checking from real device, not emulator

@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
@manishprivet

manishprivet commented Jun 22, 2021

Copy link
Copy Markdown
Member Author

I think we should merge this PR ASAP as the mobile site is broken RN. I can't seem to reproduce the issue through any means, but I still added some extra gap to the close button so hopefully, the issues mentioned are solved.

@benhalverson @rodion-arr

@github-actions

github-actions Bot commented Jun 22, 2021

Copy link
Copy Markdown

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

@benhalverson benhalverson merged commit 3e559b6 into nodejs:main Jun 22, 2021
@manishprivet manishprivet mentioned this pull request Jun 22, 2021
Merged
@LadyK-21 LadyK-21 mentioned this pull request Jun 21, 2022
Closed
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

2 more reviewers

@rodion-arr rodion-arr rodion-arr left review comments

@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.

Impossible to close menu in Learn section on mobile

4 participants

@manishprivet @codecov-commenter @rodion-arr @benhalverson