Fix bug in 'scroll into view' for storage form#5401
Fix bug in 'scroll into view' for storage form#5401akolson merged 3 commits intolearningequality:hotfixesfrom
Conversation
akolson
left a comment
akolson
left a comment
There was a problem hiding this comment.
Thanks @marcellamaki Code changes LGTM. It however appears that the other issue of the inability to click the checkboxes in the dropdown hasn't been resolved. Are we resolving it with this pr? If not, we should be good to go!
|
🤦♀️ I just completely forgot about that bit from the issue. I can add that in, thanks @akolson -- sorry I totally missed that |
akolson
added
TAG: ux update
TODO: needs review
DEV: frontend
P0 - critical
bjester
left a comment
bjester
left a comment
There was a problem hiding this comment.
Requesting a small change to match what seems to be the prior intent of the storage form multi select
| const options = sortBy(this.channels, c => c.name.toLowerCase()).filter(c => !c.public); | ||
| return options.map(option => ({ | ||
| text: option.name, | ||
| value: option.id, |
There was a problem hiding this comment.
It seems the MultiSelect component didn't accept a function for customizing itemValue, but the implementation here expected that it would. So the appropriate change would be to format the value to match what the deleted method would return.
| } else if (typeof this.itemText === 'function') { | ||
| return this.itemText(item); | ||
| } | ||
| return item.text || item; |
There was a problem hiding this comment.
I don't see any usages of this fallback behavior || item, so the template changes seem good.
| const options = sortBy(this.channels, c => c.name.toLowerCase()).filter(c => !c.public); | ||
| return options.map(option => ({ | ||
| text: option.name, | ||
| value: this.channelName(option), |
3 participants
Summary
There was existing code to scroll to the storage form when it opened, but it wasn't working. This makes a few small tweaks and now it properly scrolls
To fix the checkbox clickability issue, both
item-text,item-valueprops have been removed from theMultiSelectcomponent as they are a source of confusion both in implementation and usage. Also, the the standard contract for the items prop is below as expected by vuetify'sv-select;[ { "text": "<string to display>" "value": "<actual value of the selected item>" } ]This pr affects the search filters in the import from other channel in addition to the request form
References
Fixes #5389
Screen.Recording.2025-09-18.at.9.06.28.AM.mov
Reviewer guidance
Go to Settings > Storage and open the storage form. It should now autoscroll so that the form start is at the top of the page. Previously, the form opened below and it was hard to tell that anything happened.