-
Notifications
You must be signed in to change notification settings - Fork 46
Adjust motion-list #5701
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
Adjust motion-list #5701
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.
Generally looks good. Just one small point that I am unsure on. If my thoughts are unjustified, consider the code approved.
| if (this.isMultiSelect) { | ||
| return; |
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.
The main place it seems to be called is when clicking the motion-list state field for that motion. That seems to be the only place unless I've overlooked something.
That field ought to be unclickable during multiselect.
Is this where that is done? Because if that functionality is already implemented in another place (like the underlying list component) this upper part of the conditional would be unnecessary, since the method would never be called in multiselect mode.
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.
That field ought to be unclickable during multiselect.
it is sadly not not clickable and this is the place where it is secured that it's not if multiselect is turned on
If that should be changed it would be done in an issue which focuses that
resolves #5655
IMPORTANT:
If a user cannot see the LoS ALL LoS icons are now hidden