Improve saving indicator #3609

manuelmeister · 2023-06-30T17:41:18Z

Closes #894

output.mp4

github-actions · 2023-06-30T17:49:43Z

⛔ Feature branch deployment currently inactive.

If the PR is still open, you can add the deploy! label to this PR to trigger a feature branch deployment.

pmattmann · 2023-07-01T13:06:41Z

Already way better! <3
And I like the icon.

Some Issues:

The Multi-Select-Dropdown for ActivityResponsible does not give any response at all (what was already the case before).
Dropdonw in general - the feedback now is only give by color - and it is event hard do distinguish between blu e and green

Additional thought:
I guess users get used to autosave very quickly. Therefore the feedback could be even smaller.

carlobeltrame · 2023-07-06T11:16:55Z

frontend/src/components/activity/content/LAThematicArea.vue

-    <v-list three-line class="mx-n4">
-      <v-list-item-group>
+    <v-list three-line class="mx-n4 ec-lathematicarea">
+      <v-list-item-group multiple :value="model">


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?

usu

Like it visually, thanks.

There is some inconsistency with non-autoSave mode. Best to check https://pr3609.ecamp3.ch/controls and review the changes.

There are quite some similar components inside ApiWrapperAppend, ApiWrapperCheckboxAppend & ApiWrapperSelectAppend. Could we reuse some of the code/visuals?

DayReponsibles and ActivityResponsibles have a manual implementation of e-select. So the saving incidators don't work for them (was already the case before). Might be worth to have a separate issue to implement a similar indicator for these 2 components.

Icon: I personally relate the chosen icon more with file upload than with a general saving action. I understand that the floppy disk is very archaic, but I guess it's still recognized widely as a saving icon. Have you seen the the cloud icon on other large platforms related with "saving"?

usu · 2023-07-06T14:03:51Z

frontend/src/components/form/api/ApiSelect.vue

@@ -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" />


When I click "Retry" in the Select, the drowdown opens. Do we need to have some @click.stop somewhere?

I also noticed this in #3144 (the very last point)

This happened with the checkbox as well because of the label. I'll have a look.

usu · 2023-07-06T14:13:20Z

frontend/src/components/form/api/ApiWrapperAppend.vue

@@ -101,6 +101,10 @@ export default {
      required: true,
      type: Object,
    },
+    resettable: {


What is the purpose of this new property?

I don't want to remove the reset functionality fully, but I currently can't find a usecase where this would be helpful.

usu · 2023-07-06T14:14:34Z

frontend/src/components/form/api/ApiWrapperAppend.vue

+          </template>
+          <span>{{ $tc('global.button.tryagain') }}</span>
+        </v-tooltip>
+        <v-tooltip v-if="resettable" bottom class="ml-auto">


This breaks the activity title edit in the ScheduleEntry view (at the moment the only place where we use autoSave=false). The cancel button is not displayed.

usu · 2023-07-06T14:16:15Z

frontend/src/components/form/api/ApiWrapperSelectAppend.vue

+              <v-icon>mdi-check</v-icon>
+            </v-btn>
+          </template>
+          <span>{{ $tc('global.button.save') }}</span>


Clicking here also opens the dropdown (although, I think that was already the case before)

usu · 2023-07-06T14:20:40Z

frontend/src/components/form/api/ApiWrapperCheckboxAppend.vue

@@ -0,0 +1,64 @@
+<template>
+  <div class="ec-api-checkbox-append mt-n1">
+    <!-- Retry/Cancel button if saving failed -->


The Checkbox Append has now cancel button anymore? It's questionable, if we ever need a Checkbox in non-autosave mode. But shouldn't it have one, for consistency with the other input components?

I think for an input that is a boolean, it just doesn't make sense to have this. I've shown this to multiple people and everyone thought it would remove the whole item, instead of resetting it to the previous state.

usu · 2023-07-06T14:22:24Z

frontend/src/components/form/api/ApiCheckbox.vue

+        <ApiWrapperCheckboxAppend
+          v-if="
+            wrapper.hasServerError ||
+            (wrapper.autoSave && wrapper.dirty) ||


This should be !wrapper.autoSave.
The second part && wrapper.dirty depends on my other comment, on whether we want to display a cancel button or not.

jest would like that you don't use multi line expressions in the vue html.
please extract this into a computed property
We don't use jest anymore since #3268

you can also leave it, found the problem.
The transformation jest uses does not allow optional chaining (?.), this is the problem in PrintConfigurator.vue and Invitation.vue.

BacLuc

ok for me if the other issues are resolved

BacLuc · 2023-07-15T12:05:11Z

frontend/src/components/form/api/ApiCheckbox.vue

+        <ApiWrapperCheckboxAppend
+          v-if="
+            wrapper.hasServerError ||
+            (wrapper.autoSave && wrapper.dirty) ||


jest would like that you don't use multi line expressions in the vue html.
please extract this into a computed property
We don't use jest anymore since #3268

BacLuc · 2023-07-15T12:08:16Z

frontend/src/components/form/api/ApiCheckbox.vue

@@ -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'


you could keep a relative import here

manuelmeister · 2023-09-18T16:52:16Z

@usu the /controls site is outdated and not fully functioning anymore. (birthday and isAdmin are missing)
I'm currently fixing a problem with parsing number inputs and tried the /controls page and was disappointed that it didn't work as expected. How can I fix these options (e.g. checkbox and date controls)?

usu · 2023-09-23T06:27:05Z

@usu the /controls site is outdated and not fully functioning anymore. (birthday and isAdmin are missing) I'm currently fixing a problem with parsing number inputs and tried the /controls page and was disappointed that it didn't work as expected. How can I fix these options (e.g. checkbox and date controls)?

They have been broken for quite some time. The controls for checkbox/switch/date have been added a long time ago (3 years or so) and back then, the user endpoint had this properties just for the sake of testing these input types. There are issues and discussion on this under #1270 and https://github.com/ecamp/ecamp3/pull/673/files#r525987849.

Probably we should use other endpoints, if we want to test these input types with proper data. E.g. using the start date of the first period for the date picker? And for the checkbox/switch we could use $printYSLogoOnPicasso on the first camp.

…icator

BacLuc · 2023-12-17T16:27:27Z

You need to merge devel again after #4136

BacLuc · 2024-04-14T10:43:19Z

I would rather keep long running PR in an issue.

manuelmeister added 6 commits June 30, 2023 19:07

Fix la thematic area accessibility

d0d06eb

Move spin-only animation to global.scss

efec4bf

Provide css classes for api wrapper states

bb6c360

Improve checkbox savings indicator

1a0c144

Improve select saving indicator

90fef29

Improve saving indicator and implement resettable prop

989e33b

manuelmeister added type: Frontend UX/UI deploy! Creates a feature branch deployment for this PR labels Jun 30, 2023

carlobeltrame reviewed Jul 6, 2023

View reviewed changes

usu reviewed Jul 6, 2023

View reviewed changes

BacLuc reviewed Jul 15, 2023

View reviewed changes

BacLuc removed the deploy! Creates a feature branch deployment for this PR label Sep 5, 2023

BacLuc mentioned this pull request Sep 12, 2023

Minor UI bugs #3144

Closed

19 tasks

manuelmeister mentioned this pull request Sep 23, 2023

Optimize dev views #3838

Merged

Merge remote-tracking branch 'upstream/devel' into improve/saving-ind…

a45f1ac

…icator

manuelmeister added the deploy! Creates a feature branch deployment for this PR label Dec 15, 2023

Lint: Fix lint after merge

1bf80af

manuelmeister temporarily deployed to feature-branch December 15, 2023 17:00 — with GitHub Actions Inactive

Try another design and fix select bugs

ef8055e

manuelmeister temporarily deployed to feature-branch December 16, 2023 21:05 — with GitHub Actions Inactive

BacLuc removed the deploy! Creates a feature branch deployment for this PR label Feb 17, 2024

BacLuc mentioned this pull request Apr 14, 2024

Improve saving indicator #4971

Open

BacLuc closed this Apr 14, 2024

manuelmeister added the Cool Ideas Proof of concepts label Apr 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve saving indicator #3609

Improve saving indicator #3609

manuelmeister commented Jun 30, 2023 •

edited

Loading

github-actions bot commented Jun 30, 2023 •

edited

Loading

pmattmann commented Jul 1, 2023 •

edited

Loading

carlobeltrame Jul 6, 2023

usu left a comment

usu Jul 6, 2023

carlobeltrame Jul 6, 2023

manuelmeister Jul 12, 2023

usu Jul 6, 2023

manuelmeister Jul 12, 2023

usu Jul 6, 2023

usu Jul 6, 2023

usu Jul 6, 2023

manuelmeister Jul 12, 2023

usu Jul 6, 2023

BacLuc Jul 15, 2023 •

edited

Loading

BacLuc Jul 15, 2023

BacLuc left a comment

BacLuc Jul 15, 2023 •

edited

Loading

BacLuc Jul 15, 2023

manuelmeister commented Sep 18, 2023

usu commented Sep 23, 2023

BacLuc commented Dec 17, 2023

BacLuc commented Apr 14, 2024

Improve saving indicator #3609

Improve saving indicator #3609

Conversation

manuelmeister commented Jun 30, 2023 • edited Loading

github-actions bot commented Jun 30, 2023 • edited Loading

⛔ Feature branch deployment currently inactive.

pmattmann commented Jul 1, 2023 • edited Loading

Choose a reason for hiding this comment

usu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BacLuc Jul 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BacLuc left a comment

Choose a reason for hiding this comment

BacLuc Jul 15, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manuelmeister commented Sep 18, 2023

usu commented Sep 23, 2023

BacLuc commented Dec 17, 2023

BacLuc commented Apr 14, 2024

manuelmeister commented Jun 30, 2023 •

edited

Loading

github-actions bot commented Jun 30, 2023 •

edited

Loading

pmattmann commented Jul 1, 2023 •

edited

Loading

BacLuc Jul 15, 2023 •

edited

Loading

BacLuc Jul 15, 2023 •

edited

Loading