-
Notifications
You must be signed in to change notification settings - Fork 47
feat(UI): make small UI Tweaks #1009
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
base: dev
Are you sure you want to change the base?
Conversation
| size="md" flat dense round | ||
| @click.stop="props.expand = !props.expand" | ||
| :icon="props.expand ? 'expand_less' : 'expand_more'" | ||
| Thinker: :icon="props.expand ? 'expand_less' : 'expand_more'" |
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.
Not sure what "Thinker" is here?
| title="Plugins" | ||
| v-model:selected="selected" | ||
| @edit="router.push(`/plugins/${selected[0].id}`)" | ||
| selection="multiple" @edit="router.push(`/plugins/${selected[0].id}`)" |
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.
put on separate lines
| > | ||
| <template v-slot:header="props"> | ||
| <q-tr :props="props"> | ||
| <q-th auto-width v-if="selection === 'multiple'"> |
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.
Just a nitpick, but some of the indents on the edits in this file are off
|
Hi Colton, The multiselect/delete on the plugins page looks good as well, but it clashes with the "click to open" workflow that the rest of the pages follow. think if the team really wants the ability to multiselect, maybe we can add a small "delete multiple" checkbox somewhere in the header of the table. But I think the rest of the team will have some thoughts as to how we want to approach multi-select in general, will probably be a good topic for next call. |
I agree the team should probably weigh in on the multi-select / delete logic. I could clean up the PR to address your edits and delete the text artifact for loading though. I wonder if there is some way to put the "wait 300 ms" logic into a decorator or standalone wrapper function so that it's not repeated everywhere... maybe eventually we could enhance the logic to not show loading if the request takes less than 100ms. I'm still very new to Vue / Quasar, so I don't really know what the best pattern is here. |
I think the way you implemented the minimum 300ms loading is the most logical, even if there's some repeated code. One other idea is to implement the minimum loading time in the |
|
Sounds good! Well I deleted/reverted the code that implemented the "delete multiple" in the PluginsView, which should address all your comments. If it looks good to you, we can merge the loading functionality and the artifact task registration changes into dev |
This PR makes two small UI tweaks.
Fix display for artifact tasks to remove unused "Input Parameters":
Enhance UI by adding loading indicators:
Potential future changes we could work on after discussing with the team: