Skip to content
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

fix(settings): Provide visual feedback 4 encryption toggle #49200

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

nfebe
Copy link
Contributor

@nfebe nfebe commented Nov 11, 2024

Show visual feedback if user clicks on encryption toggle when it cannot be activated.

Demo

feedback-no-encryption-module.webm

Other improvement

  • Object.entries([]) would return an empty array, that leads to the find
  • return undefined.
  • encryptionReady and encryptionEnabled also returns undefined when no encryption module is available or functional (I guess).

Resolves : #48829

@nfebe nfebe force-pushed the fix/48829/visual-feedback-4-encryption-toggle branch from 35222b8 to 2a2f629 Compare November 11, 2024 14:32
@nfebe nfebe marked this pull request as ready for review November 11, 2024 14:58
@nfebe nfebe force-pushed the fix/48829/visual-feedback-4-encryption-toggle branch from 2731f3d to 016738f Compare November 11, 2024 14:59
Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can't we just disable the checkbox-switch and show a warning note?

@nfebe
Copy link
Contributor Author

nfebe commented Nov 11, 2024

Can't we just disable the checkbox-switch and show a warning note?

That is one option, I am just did this around the feedback from a user.... cc: @marcoambrosini @jancborchardt

Plus that has the limitation that we have to display the warning at all times somewhere on the page as disabled components do not respond to events.

@marcoambrosini
Copy link
Member

marcoambrosini commented Nov 15, 2024

Can't we just disable the checkbox-switch and show a warning note?

I think this is the way to go @nfebe

--
In general we don't want hover feedback or cursor: pointer on a switch that cannot be activated.

@nfebe
Copy link
Contributor Author

nfebe commented Nov 15, 2024

This means we would display the warning around (ontop, beside, under) the toggle? cc; @marcoambrosini

@marcoambrosini
Copy link
Member

Usually we place warning text underneath the inputs. We have this built into the password input component for example. Maybe it would even make sense to include this in the component? @skjnldsv wdyt?

@nfebe nfebe force-pushed the fix/48829/visual-feedback-4-encryption-toggle branch from 016738f to 61bac10 Compare November 22, 2024 20:23
Copy link
Contributor Author

@nfebe nfebe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning-no-encryption-module

Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi Fon,

a couple of question:
*The switch still does not look disabled can we implement that look?

  • What do you mean by "Please enable an encryption module or app"? I mean what's the difference between them?

@nfebe
Copy link
Contributor Author

nfebe commented Nov 25, 2024

*The switch still does not look disabled can we implement that look?

Would that be blurring it out? What is the look?

What do you mean by "Please enable an encryption module or app"? I mean what's the difference between them?

The encryption system allows support for different encryption modules. The "default encryption module" is implemented in the app "Default encryption module".

So in this case they are the same thing hence the use of OR.

@skjnldsv
Copy link
Member

skjnldsv commented Dec 3, 2024

Would that be blurring it out? What is the look?

use :disabled="true".
Right now it looks off but not disabled. If it cannot be enabled because encryption is not ready, it should be shown as this:
image

@skjnldsv skjnldsv added 2. developing Work in progress and removed 3. to review Waiting for reviews labels Dec 3, 2024
@marcoambrosini
Copy link
Member

The encryption system allows support for different encryption modules. The "default encryption module" is implemented in the app "Default encryption module".

Perfect, then for the text I think we should use:
"Encryption is not available. Please enable the Encryption app or an encryption module"
Both for "encription app" and "encryption module" I think we should add links to the documentation. Or 1 link for the whole sentence is also ok if we have a unified documentation about all the possibilities here.

`Object.entries([])` would return an empty array, that leads to the `find`
return undefined.

`encryptionReady` and `encryptionEnabled` also returns undefined when no
encryption module is available or functional (I guess).

Signed-off-by: nfebe <fenn25.fn@gmail.com>
@nfebe
Copy link
Contributor Author

nfebe commented Dec 3, 2024

use :disabled="true".
Right now it looks off but not disabled. If it cannot be enabled because encryption is not ready, it should be shown as this:

Well, I was asking because that is what was there before this was ever a pull request and my initial changes removed that, and then added the timeout effect but it as was restored and just the notice added under the toggle.

:disabled="encryptionEnabled"

Show visual feedback if user clicks on encryption toggle when it cannot be activated.

Signed-off-by: nfebe <fenn25.fn@gmail.com>
@nfebe nfebe force-pushed the fix/48829/visual-feedback-4-encryption-toggle branch from 61bac10 to b374055 Compare December 3, 2024 14:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 🏗️ In progress
Development

Successfully merging this pull request may close these issues.

enh(Settings): Provide visual feedback to user when encryption app not installed.
3 participants