SvelteKit: Add 'Tools' Menu item back to global navigation#64134
SvelteKit: Add 'Tools' Menu item back to global navigation#64134jasonhawkharris merged 5 commits intomainfrom
Conversation
6f28178 to
0e2ab8c
Compare
0e2ab8c to
cd64962
Compare
There was a problem hiding this comment.
non-blocking: I think we can just remove href from NavigationMenuDefinition entirely and just create button elements for the menu now. That probably also means removing the "on hover" behavior to match the react app.
Mind making a followup ticket for this? I think it's fine to merge this as-is since this is changing so fast.
|
When I click the Tools menu, page elements "flash" - it looks like it's interacting with the whole page, or causing a reload or something. Is that by design? Screen.Recording.2024-07-29.at.10.12.36.mp4 |
|
@peterguy, yes that is intentional, at the moment. Is it okay to merge as is? I can make a follow up issue for this and fix it before the end of the week. I just don't want to get to granular on this while I'm trying to complete some other stuff. |
|
Yep, merge it and followup! I saw @camdencheek 's comment about converting to buttons after I had posted my comment, or I wouldn't have posted. |
cd64962 to
783d5e3
Compare
…age (#64136) <!-- PR description tips: https://www.notion.so/sourcegraph/Write-a-good-pull-request-description-610a7fd3e613496eb76f450db5a49b6e --> When clicking a menu item with no href, page refreshed every time. Now, menu items with no href cause no action when clicked. https://github.com/user-attachments/assets/abcc5164-64fa-4b37-8f8c-7df110bca23e This is a follow up to this PR: https://github.com/sourcegraph/sourcegraph/pull/64134 ## Test plan Manual/Visual testing <!-- REQUIRED; info at https://docs-legacy.sourcegraph.com/dev/background-information/testing_principles --> ## Changelog <!-- OPTIONAL; info at https://www.notion.so/sourcegraph/Writing-a-changelog-entry-dd997f411d524caabf0d8d38a24a878c -->
This PR adds the
ToolsMenu Item back to the Global Navigation Bar of the svelte-kit web app, and removes the sub menus for bothCode SearchandCodymenu items.Before:

After:

Test plan
Visual/manual testing
Passing CI
Changelog