Improve storyboard ux #3224
Improve storyboard ux #3224
Conversation
manuelmeister
commented
Jan 7, 2023
manuelmeister
commented
| before | after |
|---|---|
 |
 |
 |
 |
✅ Deploy Preview for ecamp3-frontend-preview ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
There was a problem hiding this comment.
Putting time and responsible on a separate row improves the look and feel on mobile a lot.
I am not sure if removing the arrows on desktop is the right way, but you will have you reasons.
But on desktop, the drag handle and the delete button is now close.
On the print ui, you put the drag handle on the left side, maybe to avoid that problem.
| <card-content-node v-resizeobserver.debounce="onResize" v-bind="$props"> | ||
| <component :is="variant === 'default' ? 'table' : 'div'"> | ||
| <thead v-if="variant === 'default'"> | ||
| <tr> |
There was a problem hiding this comment.
We really want this to be a normal table and not v-row/v-col components as before?
There was a problem hiding this comment.
Pretty sure. Otherwise we need to reimplement a11y for the whole thing again.
There was a problem hiding this comment.
I'm unfortunately not really knowledgeable an a11y, so cannot really judge on this. What seems strange to me is that we use table elements for something that doesn't really look like a table (in dense variant) and where vuetify provided the v-row/v-col for exactly this purpose.
Also in default variant there's an extra div in the table:
And in dense variant there's even no table element:
As far as I know, both are not valid HTML.
There was a problem hiding this comment.
This should be sorted now
Core Meeting Decision
|
# Conflicts: # frontend/package-lock.json # frontend/package.json
⛔ Feature branch deployment currently inactive.If the PR is still open, you can add the |
There was a problem hiding this comment.
Just checked visually, haven't looked into the code yet. Visual appearance looks good to me. Points below are the ones I noticed so far.
-
Without any content, all the columns are smashed together.
-
In dense variant, the tab order is still time -> program -> responsible, which is visually unexpected
-
Arrows are completely gone. By purpose? Cannot fully recall the discussion in January, but the notes say "Up and down only on focus".
|
There is also this sentry issue which popped up on the feature branch deployment: https://ecamp.sentry.io/issues/3980035515/ |
I thought we had configured that it always needs at least one row.
I agree, but taborder should not be changed. Also it is not easily changeable via vuetify. I added a if statement to combat this.
Yes, I changed it to be movable with the arrow keys while on the move handle. This way, it is a power user feature and does not disturb the tabflow anymore than the move handle and the delete button already do. I tried with an overlay but it required far more work. I could put in in a new issue, if the new way is not the preferred way. |
Ah yes, true. In this case more of an edge case with our dev data. |
| class="drag-and-drop-handle" | ||
| :disabled="isLastSection" | ||
| :aria-label="$tc('components.activity.content.storyboardRow.move')" | ||
| @keydown.down="$emit('moveDown', itemKey)" |
There was a problem hiding this comment.
Ah I see, here are the keyup/keydown. Definitely a "power-user" thing and didn't work well for me even after knowing the feature. I thought we wanted to keep the arrows especially for mobile, where this hack doesn't work. But also for this, I'm ok to merge and we can improve/discuss later.
There was a problem hiding this comment.
I agree. I think this power feature would need some work anyway, because if you move an entry down, the focus should stay on the handle. We could also show a hint when the handle ist focused that you can move the entry with the arrow keys.
We tried tried drag&drop on different mobile platforms and were surprised that it worked so well. So we decided to use the arrows only on focus. But in my experiments I made an overlay over the whole row that was only visible when focused and this felt strange and cumbersome, as it introduced two tabstops per row that are initially hidden. I think we can find out in the user feedback.
|
@carlobeltrame the issue you've described is a performance problem and probably needs to be looked at. However it is not critical as it just doesn't update to the other variant. |
There was a problem hiding this comment.
Love it! I really like how it even looks great on desktop if I have a narrow storyboard (in a narrow column of a columnLayout).
The reordering and maybe deleting could use a save progress indicator some time in the future.
As a guest in the camp, I still see the drag handles and delete buttons, but when I try to use them I get Access Denied toasts. Similarly, If I try to edit the storyboard sections, our red retry and cancel buttons appear. This is fine for now, but maybe we can disable the controls or simplify the view for guests in the future.
The css violates vue-scoped-css/require-v-deep-argument which will be more strictly enforced by vue3. I changed all the values to 10'000, and it had no effect. Because there was no explanation why this is here in the commit message, in the PR or in the comments, i delete this css. Was introduced in: PR: ecamp#3224 Commit: 9060a33
The css violates vue-scoped-css/require-v-deep-argument which will be more strictly enforced by vue3. I changed all the values to 10'000, and it had no effect. Because there was no explanation why this is here in the commit message, in the PR or in the comments, i delete this css. Was introduced in: PR: ecamp#3224 Commit: 9060a33
