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

Adjust side-nav-more-options to account for anchor style #3097

Conversation

MarmadileManteater
Copy link
Contributor

@MarmadileManteater MarmadileManteater commented Jan 21, 2023

Adjust side-nav-more-options to account for difference in style between router-link and div

Pull Request Type

  • Bugfix
  • Feature Implementation
  • Documentation
  • Other

Description

Somewhat recently, the side-nav-more-options icons were changed to be router-links instead of divs, but the style difference between router-link and div still needs to be accounted for because router-links carry with them all of the styles of anchors. Due to this, the text in the more options menu is the primary link color and the background color is not visible on hover (due to the display being inline). This PR aims to address this by adding some of the styles that divs have to the .navOption class in the side-nav-more-options component.

Screenshots

Before After
before after

Testing

  1. Make sure the window is narrow enough that the More navbar item appears: (width < 678px).
  2. Click the More navbar item.
  3. Ensure that the navbar options under More are do not appear to be the primary link color
  4. Also, ensure that they have a background color when you hover your mouse over them.

Desktop

  • OS: Windows 10
  • OS Version: Pro Version 21H2 Installed on ‎4/‎3/‎2022 OS build 19044.1889 Experience Windows Feature Experience Pack 120.2212.4180.0
  • FreeTube version: 0.18.0

- These nav-options were changed from `div`s to anchors, and so,
they need to be styled like the `div`s that used to be there.
@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 21, 2023
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) January 21, 2023 00:27
@MarmadileManteater MarmadileManteater marked this pull request as draft January 21, 2023 00:29
auto-merge was automatically disabled January 21, 2023 00:29

Pull request was converted to draft

@MarmadileManteater MarmadileManteater marked this pull request as ready for review January 21, 2023 00:34
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) January 21, 2023 00:35
@MarmadileManteater MarmadileManteater marked this pull request as draft January 21, 2023 03:00
auto-merge was automatically disabled January 21, 2023 03:01

Pull request was converted to draft

Copy link
Collaborator

@PikachuEXE PikachuEXE left a comment

Choose a reason for hiding this comment

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

image

@MarmadileManteater MarmadileManteater marked this pull request as ready for review January 21, 2023 06:58
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) January 21, 2023 06:58
@FreeTubeBot FreeTubeBot merged commit 7ccb5dd into FreeTubeApp:development Jan 21, 2023
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Jan 21, 2023
MarmadileManteater added a commit to MarmadileManteater/FreeTubeAndroid that referenced this pull request Jan 22, 2023
commit bed2485
Merge: c7e7e7e 7ccb5dd
Author: Emma <MarmadileManteater@proton.me>

Merge remote-tracking branch 'upstream/development' into development

commit c7e7e7e
Author: Emma <MarmadileManteater@proton.me>

Base Cordova build off of upstream web build (#84)

commit 7ccb5dd
Author: Emma <MarmadileManteater@proton.me>

Adjust `side-nav-more-options` to account for anchor style (FreeTubeApp#3097)

...

**Full Changelog**: 0.18.0-nightly-80...0.18.0-nightly-81
@MarmadileManteater MarmadileManteater deleted the minor-css-fix-to-sidenav branch March 16, 2024 15:57
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.

None yet

5 participants