-
Notifications
You must be signed in to change notification settings - Fork 24
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
Show dataset name or explorative tracing name in webknossos tab title #4767
Conversation
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.
Awesome, feels really cool! I left some smaller suggestions though :)
Also pinging @daniel-wer since he had the idea about this feature.
@@ -159,6 +162,8 @@ class TracingLayoutView extends React.PureComponent<PropsWithRouter, State> { | |||
storeLayoutConfig(this.currentLayoutConfig, layoutKey, this.currentLayoutName); | |||
}; | |||
|
|||
getTabTitle = () => `${this.props.displayName} ${this.props.organization} webknossos`; |
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.
getTabTitle = () => `${this.props.displayName} ${this.props.organization} webknossos`; | |
getTabTitle = () => `${this.props.displayName} ${this.props.organization} webKnossos`; |
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.
Also, I think that some sort of separator between these different parts would be nice. For example an |
. If some of these parts happens to be empty or null, unnecessary |
should be avoided then (for example, filtering the parts as an array and then use join
).
@@ -343,6 +349,8 @@ function mapStateToProps(state: OxalisState): StateProps { | |||
storedLayouts: state.uiInformation.storedLayouts, | |||
isDatasetOnScratchVolume: state.dataset.dataStore.isScratch, | |||
datasetName: state.dataset.name, | |||
displayName: state.tracing.name ? state.tracing.name : state.dataset.name, | |||
organization: state.activeUser ? state.activeUser.organization : "", |
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.
Does this mean that no organization is shown if the user is not logged in? I think, this is problematic since the organization title should especially be presented for guest users.
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.
Should it be the organization that owns the dataset instead?
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.
Yes, I think that makes sense!
Very cool, works well for me! Agree with @philippotto's feedback 👍 |
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.
Excellent! Just make sure to fix the changelog before merging :)
CHANGELOG.unreleased.md
Outdated
@@ -24,6 +24,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.released | |||
- Improved the performance of applying agglomerate files. [#4706](https://github.com/scalableminds/webknossos/pull/4706) | |||
- In the Edit/Import Dataset form, the "Sharing" tab was renamed to "Sharing & Permissions". Also, existing permission-related settings were moved to that tab. [#4683](https://github.com/scalableminds/webknossos/pull/4763) | |||
- Improved handling and communication of failures during download of data from datasets. [#4765](https://github.com/scalableminds/webknossos/pull/4765) | |||
- The brush tool in hybrid tracings is disabled while in merger mode. [#4757](https://github.com/scalableminds/webknossos/pull/4770) |
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 think this ended up in the wrong branch 😄
This PR displays the dataset or explorative tracing name in the tab while annotating.
URL of deployed dev instance (used for testing):
Steps to test:
<dataset_name> <organization_name> webKnossos
Issues:
Updated (unreleased) changelogUpdated (unreleased) migration guide if applicableUpdated documentation if applicableAdapted wk-connect if datastore API changesNeeds datastore update after deployment