-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
feat(unify): launchpad header breadcrumbs and reusable tooltip component #20648
Merged
mapsandapps
merged 43 commits into
10.0-release
from
mapsandapps/feat/launchpad-header-breadcrumbs
Apr 19, 2022
Merged
Changes from 4 commits
Commits
Show all changes
43 commits
Select commit
Hold shift + click to select a range
844edb8
display testing type in breadcrumb
8391e10
add component tests
91c71f5
add e2e tests
0a86fa7
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
0244bff
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
c7dbb56
improve navigation
085bab7
prevent navigation while loading
2379000
update test
6730945
refactor for cleaner code
32a912b
fix ts mistake
3c2f033
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
dbae6d8
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
1b9c1c7
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
e214231
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
20abd8c
Truncate branch
151ffa1
Tooltip
e99c8aa
Create tooltip component
cab9327
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
8ca09c7
Animation
cb30602
Replace SidebarNavigationRow tooltip
b27c3c8
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
ba242c7
Animation for placement right
3562b74
Animation for other placements
8f3b155
Delete now-unused SidebarTooltip
0329420
Replace SelectorPlaygroundTooltip
03ef886
Replace UnsupportedBrowserTooltip
5296f9c
Windify
bc43a34
Appease check-ts
bda72b6
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
b910768
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
dd01555
Add tests
845b877
appease TS
73e2cef
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
36d1800
push pixels
f19bf5d
remove unused divs
ac80921
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
4705255
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
8e948e5
Cause tooltip to be less persistent
1e477c3
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
ac8b72a
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
c1d30a3
Fix merge issues
b63834d
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
18cf600
Merge branch '10.0-release' into mapsandapps/feat/launchpad-header-br…
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For accessibility reasons, we should make this either a link or a button so it can receive keyboard focus and be announced the right way. Before it becomes interactive, the disabled link pattern here seems like it would work:
I think we should follow this pattern for the Projects link as well, we are already close to it there - though that doesn't need to get updated in this PR.
The "link with a fake href because it actually uses a click handler to trigger a mutation that updates state that will move the UI to a new 'page'" doesn't seem ideal at first, but it may be the clearest pattern to understand from a user perspective, in terms of how these breadcrumbs behave.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't think you should ever have an
a
without anhref
, so this was news to me!In my update, I went ahead and used
<nav>
and<ol>
for this, which i had been intending to do if i made a reusable breadcrumb component. LMK what you think about this.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice, I think that's good. I had only found the disabled link pattern recently, I think we needed it somewhere else in launchpad as well.