-
Notifications
You must be signed in to change notification settings - Fork 3
feat: bookmarks sidebar #277
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
Conversation
This reverts commit 1767e9a.
@@ -5,5 +5,5 @@ export default config({ | |||
reporter: [['list'], ['html', { outputFolder: '../test-reports', open: 'never' }], ['github']], | |||
retries: 1, | |||
forbidOnly: true, | |||
workers: 1, | |||
workers: 2, |
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.
gotta go fast, but in CI :)
miguel this is really good!! looks like bookmarks are being stored correctly in the sidebar. my main feedback is that it seems clicking on a bookmark doesn't take me to the metric the rest of my feedback is non-blocking nits:
277.toast-notif.mov277.scrollbar-persist.mov |
clicking on a bookmark now takes me to the bookmarked metric - nice!! ❤️ i noticed a few other things. since i think the MVP functionality of bookmarks has been met (ability to bookmark, click on a bookmark and see the metric), i think these things can be addressed in the future. WDYT?
277.filters-on-top-after-bookmarking.mov
277.clicking-on-bookmark-no-star.mov
277.cross-DS-bookmarks.mov |
This looks good! Two small nits:
|
re:
This was my bad! Fixed by #326. |
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.
Looking good!
Later, let's figure out how to remove the special navigation event handling introduced by this PR.
Co-authored-by: Nick Richmond <[email protected]> Signed-off-by: Miguel Palau <[email protected]>
Fixes: #253
Empty: