Skip to content
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

Scroll faster through tree comments #3041

Merged
merged 8 commits into from
Aug 13, 2018

Conversation

MichaelBuessemeyer
Copy link
Contributor

@MichaelBuessemeyer MichaelBuessemeyer commented Aug 13, 2018

URL of deployed dev instance (used for testing):

  • https://___.webknossos.xyz

Steps to test:

  • open the tracing view
  • create/have many trees with comments
  • go to the comment tab and scroll though them with n and p
  • holding the keys pressed to scroll permanent
  • the delay between pressing the key and the actual start of the scrolling should match the users' delay

Issues:


Copy link
Member

@philippotto philippotto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice 👍 See my one comment before squashing into master, though :)

CHANGELOG.md Outdated
@@ -13,6 +13,7 @@ For upgrade instructions, please check the [migration guide](MIGRATIONS.md).

- Added two new properties to mapping json files. The `colors: [<hsvHueValue1>, <hsvHueValue2>, ...]` property can be used to specify up to 256 custom colors for the first 256 equivalence classes of the mapping. The `hideUnmappedIds: <true|false>` property indicates whether segments that were not mapped should be rendered transparently or not. [#2965](https://github.com/scalableminds/webknossos/pull/2965)
- Added a button for refreshing the dataset in the backend cache. [#2975](https://github.com/scalableminds/webknossos/pull/2975)
- Comments of tracing trees are now scrollable by holding n and p pressed. [#3041](https://github.com/scalableminds/webknossos/pull/3041)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Scrollable does not describe this perfectly, I'd argue. I would use "cycle" instead:

Comments of tracing trees can now be cycled through by keeping n and p pressed.

@@ -85,6 +87,7 @@ type CommentTabStateType = {

class CommentTabView extends React.PureComponent<Props, CommentTabStateType> {
listRef: ?List;
storePropertyUnsubscribers: Array<Function>;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Function is a very generic flow type and should be avoided when possible, since it lacks parameter and return types. Just write () => void instead.

@@ -105,6 +108,11 @@ class CommentTabView extends React.PureComponent<Props, CommentTabStateType> {
return { data };
}

constructor(...args: any) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You don't need the constructor for this static assignment, instead you can do this right in line 90, as follows:
storePropertyUnsubscribers: Array<Function> = [];

@daniel-wer
Copy link
Member

@MichaelBuessemeyer I'd say in the future it is enough to assign one reviewer, otherwise you may end up with duplicate reviews :)

@MichaelBuessemeyer
Copy link
Contributor Author

@daniel-wer looks like a missclick to me 🙈

@MichaelBuessemeyer MichaelBuessemeyer merged commit cead815 into master Aug 13, 2018
@philippotto philippotto deleted the scroll-faster-through-tree-comments branch January 24, 2019 08:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Long press P and N to speed through comments
3 participants