-
Notifications
You must be signed in to change notification settings - Fork 50
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
Improve saving indicator #3609
Improve saving indicator #3609
Changes from 6 commits
d0d06eb
efec4bf
bb6c360
1a0c144
90fef29
989e33b
a45f1ac
1bf80af
ef8055e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,14 @@ Displays a field as a e-checkbox + write access via API wrapper | |
@input="wrapper.on.input" | ||
> | ||
<template #append> | ||
<api-wrapper-append :wrapper="wrapper" /> | ||
<ApiWrapperCheckboxAppend | ||
v-if=" | ||
wrapper.hasServerError || | ||
(wrapper.autoSave && wrapper.dirty) || | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you can also leave it, found the problem. |
||
wrapper.hasLoadingError | ||
" | ||
:wrapper="wrapper" | ||
/> | ||
</template> | ||
</e-checkbox> | ||
</api-wrapper> | ||
|
@@ -25,11 +32,11 @@ Displays a field as a e-checkbox + write access via API wrapper | |
<script> | ||
import { apiPropsMixin } from '@/mixins/apiPropsMixin.js' | ||
import ApiWrapper from './ApiWrapper.vue' | ||
import ApiWrapperAppend from './ApiWrapperAppend.vue' | ||
import ApiWrapperCheckboxAppend from '@/components/form/api/ApiWrapperCheckboxAppend.vue' | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. you could keep a relative import here |
||
|
||
export default { | ||
name: 'ApiCheckbox', | ||
components: { ApiWrapper, ApiWrapperAppend }, | ||
components: { ApiWrapperCheckboxAppend, ApiWrapper }, | ||
mixins: [apiPropsMixin], | ||
props: { | ||
// disable delay per default | ||
|
@@ -41,4 +48,58 @@ export default { | |
} | ||
</script> | ||
|
||
<style scoped></style> | ||
<style scoped lang="scss"> | ||
.ec-api-wrapper--saving ::v-deep .v-input--selection-controls__input .v-icon { | ||
transition: color 0.2s ease; | ||
color: rgba(0, 0, 0, 0.5) !important; | ||
} | ||
|
||
.ec-api-wrapper ::v-deep .v-input--checkbox .v-input--selection-controls__input::before { | ||
border: 2px solid transparent; | ||
border-radius: 50%; | ||
position: absolute; | ||
height: 24px; | ||
width: 24px; | ||
content: '\F0131'; | ||
color: transparent; | ||
transition: transform 0.2s ease-out, border-color 0.2s ease; | ||
transform: scale(1.9); | ||
pointer-events: none; | ||
} | ||
|
||
.ec-api-wrapper--saving | ||
::v-deep | ||
.v-input--checkbox | ||
.v-input--selection-controls__input::before { | ||
border-color: map-get($blue, 'base'); | ||
-webkit-mask: conic-gradient(from 0deg at 50% 50%, #0000 0%, #000); | ||
transform: scale(1.2); | ||
animation: spin 1s linear infinite both; | ||
transition: transform 0.2s ease-out; | ||
} | ||
|
||
@keyframes spin { | ||
from { | ||
transform: scale(1.5) rotate(0deg); | ||
} | ||
to { | ||
transform: scale(1.5) rotate(360deg); | ||
} | ||
} | ||
|
||
.ec-api-wrapper--success | ||
::v-deep | ||
.v-input--checkbox | ||
.v-input--selection-controls__input::before { | ||
border-color: map-get($green, 'lighten-2'); | ||
transform: scale(1.5); | ||
transition: none; | ||
} | ||
|
||
.ec-api-wrapper--server-error | ||
::v-deep | ||
.v-input--checkbox | ||
.v-input--selection-controls__input::before { | ||
transform: scale(1.5); | ||
} | ||
</style> |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,7 +15,7 @@ Displays a field as a e-select + write access via API wrapper | |
:readonly="wrapper.readonly" | ||
:disabled="disabled" | ||
:error-messages="wrapper.errorMessages" | ||
:loading="wrapper.isSaving || wrapper.isLoading ? 'secondary' : false" | ||
:loading="wrapper.isLoading ? 'secondary' : false" | ||
:outlined="outlined" | ||
:filled="filled" | ||
:dense="dense" | ||
|
@@ -24,7 +24,7 @@ Displays a field as a e-select + write access via API wrapper | |
> | ||
<template #append> | ||
<v-icon>mdi-menu-down</v-icon> | ||
<api-wrapper-append :wrapper="wrapper" /> | ||
<ApiWrapperSelectAppend :wrapper="wrapper" :resettable="resettable" /> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When I click "Retry" in the Select, the drowdown opens. Do we need to have some @click.stop somewhere? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I also noticed this in #3144 (the very last point) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This happened with the checkbox as well because of the label. I'll have a look. |
||
</template> | ||
</e-select> | ||
</api-wrapper> | ||
|
@@ -33,11 +33,11 @@ Displays a field as a e-select + write access via API wrapper | |
<script> | ||
import { apiPropsMixin } from '@/mixins/apiPropsMixin.js' | ||
import ApiWrapper from './ApiWrapper.vue' | ||
import ApiWrapperAppend from './ApiWrapperAppend.vue' | ||
import ApiWrapperSelectAppend from '@/components/form/api/ApiWrapperSelectAppend.vue' | ||
|
||
export default { | ||
name: 'ApiSelect', | ||
components: { ApiWrapper, ApiWrapperAppend }, | ||
components: { ApiWrapperSelectAppend, ApiWrapper }, | ||
mixins: [apiPropsMixin], | ||
props: { | ||
// disable delay per default | ||
|
@@ -62,4 +62,8 @@ export default { | |
} | ||
</script> | ||
|
||
<style scoped></style> | ||
<style scoped> | ||
::v-deep .v-input__append-inner { | ||
position: relative; | ||
} | ||
</style> |
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.
Is v-list-item-group really the correct component to use here? When navigating via keyboard, I can now focus either the checkbox or the whole list item. If I try to "activate" the list item via spacebar, it plays an animation but has no effect on the checkbox. Could you either make sure that only the checkboxes are focusable, or at least make the list item toggle the checkbox when "clicked" via keyboard?