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

fix: Explore hover dropdown doing weird things #6430

Merged
merged 2 commits into from
Jul 20, 2023

Conversation

Jarsen136
Copy link
Contributor

PR Type

  • Bugfix
  • Feature
  • Refactoring

Needs Design check

Context

@Jarsen136 Jarsen136 requested a review from a team as a code owner July 13, 2023 16:28
@Jarsen136 Jarsen136 requested review from preschian and roiLeo and removed request for a team July 13, 2023 16:28
@kodabot
Copy link
Collaborator

kodabot commented Jul 13, 2023

WARNING @Jarsen136 PR for issue #6427 which isn't assigned to you. Please be warned that this PR may get rejected if there's another assignee for issue #6427

@netlify
Copy link

netlify bot commented Jul 13, 2023

Deploy Preview for koda-canary ready!

Name Link
🔨 Latest commit 7a5b090
🔍 Latest deploy log https://app.netlify.com/sites/koda-canary/deploys/64b7d320c040590008b010bf
😎 Deploy Preview https://deploy-preview-6430--koda-canary.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@reviewpad
Copy link
Contributor

reviewpad bot commented Jul 13, 2023

AI-Generated Summary: This pull request makes changes to three files: ExploreDropdown.vue, NavbarExploreOptions.vue, and NeoDropdown.scss. The patches primarily aim to fix issues related to hover dropdown in the navigation bar. In ExploreDropdown.vue, the trigger template is modified and a div element has been added. Changes in NavbarExploreOptions.vue involve an additional div element for better alignment and structuring. In the NeoDropdown.scss file, a margin property is removed from the dropdown menu styling. Overall, these patches seem to enhance navigation bar functionality and improve UI/UX.

@reviewpad reviewpad bot added small Pull request is small waiting-for-review labels Jul 13, 2023
@Jarsen136
Copy link
Contributor Author

There is a margin gap between the button and the dropdown menu, which would make the menu hard to hover. So I remove the gap in this PR.

image

@prury prury added the S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked label Jul 18, 2023
Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

There is a margin gap between the button and the dropdown menu, which would make the menu hard to hover. So I remove the gap in this PR.
image

Nice but you remove the gap for all dropdown...

Capture d’écran 2023-07-18 à 5 27 03 PM

components/navbar/ExploreDropdown.vue Show resolved Hide resolved
@roiLeo roiLeo added the S-changes-requested-🤞 PR is almost good to go, just some fine tunning label Jul 18, 2023
@Jarsen136
Copy link
Contributor Author

Jarsen136 commented Jul 18, 2023

There is a margin gap between the button and the dropdown menu, which would make the menu hard to hover. So I remove the gap in this PR.

Nice but you remove the gap for all dropdown...

I have tried to apply the change to that dropdown but failed. The style would always apply to all dropdown.
$dropdown-menu-margin

@prury prury removed the S-works-for-me-✅ qa-guild has tested PR from end user perspective and functionality worked label Jul 18, 2023
@roiLeo
Copy link
Contributor

roiLeo commented Jul 19, 2023

I have tried to apply the change to that dropdown but failed. The style would always apply to all dropdown. $dropdown-menu-margin

for example this would work:

:deep .navbar-explore .o-drop__menu {
  margin: 0;
}
Capture d’écran 2023-07-19 à 8 32 59 AM

Maybe try to add height: 100% to .o-drop and then flex, align-items: center

@codeclimate
Copy link

codeclimate bot commented Jul 19, 2023

Code Climate has analyzed commit 7a5b090 and detected 0 issues on this pull request.

View more on Code Climate.

@Jarsen136
Copy link
Contributor Author

I have tried to apply the change to that dropdown but failed. The style would always apply to all dropdown. $dropdown-menu-margin

for example this would work:

:deep .navbar-explore .o-drop__menu {
  margin: 0;
}
Capture d’écran 2023-07-19 à 8 32 59 AM Maybe try to add `height: 100%` to `.o-drop` and then flex, align-items: center

Thanks. It works well to remove the margin gap from the dropdown. I have update the code.

@Jarsen136 Jarsen136 requested a review from roiLeo July 19, 2023 12:27
Copy link
Contributor

@roiLeo roiLeo left a comment

Choose a reason for hiding this comment

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

I don't like to see fixed width/height but it's working as excepted

✅ code lgtm

@roiLeo roiLeo added 🧪 - tested PR was tested and works as expected and removed S-changes-requested-🤞 PR is almost good to go, just some fine tunning labels Jul 20, 2023
@Jarsen136 Jarsen136 merged commit 853fc67 into kodadot:main Jul 20, 2023
19 checks passed
This was referenced Jul 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🧪 - tested PR was tested and works as expected small Pull request is small waiting-for-review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Explore hover dropdown doing weird things
5 participants