[WIP] Status bar #5369
Merged
[WIP] Status bar #5369
Conversation
This file contains hidden or 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
philippotto
reviewed
Apr 8, 2021
There was a problem hiding this comment.
Cool stuff 💯 Awesome to see it merged with the layout pr. The code looks pretty good 👍 I only left some small code-related comments.
Other than that I would on the following before we can merge this:
- the shortcut hints should adapt to the current tool. I might have said in an earlier talk, that this is not high-pri for now, but I think, this would need to change to get this merged, since the statusbar would contain wrong information otherwise (e.g., when being in brush-mode, the status bar tells the user that they can move with the left mouse button).
- the icons need better contrast/colors
- the alignment might need to be double checked
You can also go through the other internal feedback you got and follow through with that, but the above points would be my personal prioritization :)
|
Thanks for your feedback! I implemented it as well as a lot of the others' feedback. |
philippotto
approved these changes
Apr 19, 2021
There was a problem hiding this comment.
Awesome! This is really cool :) I only left two small to dos, but afterwards, this should be good to merge into @MichaelBuessemeyer's layout PR 👍
| flexWrap: "wrap", | ||
| overflow: "hidden", | ||
| }; | ||
| const defaultIconStyle = { height: fontSize - 1, marginTop: 2 }; |
There was a problem hiding this comment.
Why fontSize - 1? At least on my screen, this makes the icons quite blurry. For example, the "movement" dashes next to the left-click-drag-icon becomes barely visible. When I bumped the height via the devtools, the icon became clear again and the height also looked alright. Or do you see any downsides to this?
Comment on lines
109
to
119
| {this.props.isSkeletonAnnotation && ( | ||
| <span style={defaultShortcutStyle}> | ||
| <img | ||
| className="keyboard-mouse-icon" | ||
| src="/assets/images/icon-statusbar-mouse-right.svg" | ||
| alt="Mouse Left" | ||
| style={defaultIconStyle} | ||
| /> | ||
| Place Node | ||
| </span> | ||
| )} |
There was a problem hiding this comment.
This part also needs to be adapted for the current tool (since one cannot place nodes in the brush tool). I'd recommend a simple object which maps from tool to right-click-action-description à la:
const rightClickToLabel = {
MOVE: isSkeletonAnnotation ? "Place Node" : null,
BRUSH: "Erase",
TRACE: "Erase",
FILL_CELL: null,
PICK_CELL: null,
}
that way, we can easily add "open context menu" as soon as we support that (instead of null).
|
🚀 💯 |
philippotto
added a commit
that referenced
this pull request
Apr 20, 2021
* WIP adapting to flex layout * WIP make sure that canvas is visible * First working version with minimal feature set * adjust layout a bit * WIP cleaning up default layout and adding default layouts for all possible base layouts * finish the builder methods for default layouts * WIP migrating default layout * finish adding arbitray default layout * Finish adding layout persistence * fix linting * remove logging and add todos * fixing layouting bug for maximize and general layout changes * add name and shot name to OrthoViews * adjust titles of viewports * css rework and various fixes * fix linting and flow * add layout reset; add error toasts; add useful info to footer * fiy arbitrary layout & do not intially render content of hidden viewports * undo making arbitrary viewport quadratic with css only * also restrict tab arrangement after layout change * fix picking the correct last active layout * setting viewport rect to empty if viewport is not rendered * set default viewport rect to 0 and fix tests * auto hide sidebars on maximize; add maximize shortcut * add sidebar buttons to navbar & maintain border status correctly while maximized * restore old settings toggle button in navbar * remove tab title border on hover * reduce header height * auto hide sidebar is width gets too small * consistent renaming of sidebar to border * remove golden layout * polish the code * remove buttons from navbar; recenter logo; change toggle button icon * final adjustments before first review * apply feedback * add border toggle buttons to top of each side * WIP apply feedback && better typing of laoyut configs * finish typing of json format * better typing for webpack config * apply feedback && do not react to tab maximizing in borders * finish last feedback * do not react to action of sublayouts used for borders * add border toggle buttons to header in tab * update expected zoom values slightly * enforce entering of icon in border toggle button * incease visibility of resize bars * undo change to expected zoom values * don't collapse borders below a certain width * set border bar siye to 1 and fix bug not rendering border toggle buttons * use our icons * reduce splitter size and increase header height for borders * fix linting * reduce splitter width to 1px * enforce left border to alwazs have the same width * increase layout version * fix bug in safari having tracing view heihgt of 0 * apply feedback * remove kl zooming * fix changes in the border not being persisted * Status bar (#5369) * add footer to tracing view * update changelog * cell id, mag and pos in status bar * shortcuts * integrate statusbar in layout rework * keyboard icons in statusbar * fix statusbar with missing seg layer * fix lint * use space * alt zoom shortcut * add custom statusbar icons * wrap single elems * align shortcuts right * fix layout * tooltip to more shortcut button * adjust icon analog with tool * implement pr feedback * icons bigger and fixed right click shortcut Co-authored-by: Florian M <[email protected]> * adapt statusbar to arbitrary mode (don't show mouse position, cell id) and show other shortcuts * add workaround for safari where tabs weren't easily clickable * update changelog * fix hotfix Co-authored-by: grittaweisheit <[email protected]> Co-authored-by: Florian M <[email protected]> Co-authored-by: Philipp Otto <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR adds a status bar to a tracing view to display important information and shortcuts.
URL of deployed dev instance (used for testing):
Steps to test:
Issues:
Updated (unreleased) migration guide if applicableUpdated documentation if applicableAdapted wk-connect if datastore API changesNeeds datastore update after deployment