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

The undo button is too far from reset button #309

Open
xnuk opened this issue Aug 12, 2023 · 10 comments · May be fixed by #312
Open

The undo button is too far from reset button #309

xnuk opened this issue Aug 12, 2023 · 10 comments · May be fixed by #312
Assignees
Labels
bug Something isn't working help wanted Extra attention is needed usability/UX

Comments

@xnuk
Copy link

xnuk commented Aug 12, 2023

Related repository: TinyWebEx/AutomaticSettings

Bug description

out.mp4

Let's say I made a mistake that I accidentally pressed the reset button. I expected some confirmation UI appears, but there was nothing. So I have to undoing options manually from my memory, and while doing it, I find there is actually 'Undo' button at the top of the settings page.

Steps to reproduce

  1. In the settings page, click 'Reset all settings to defaults'
  2. No confirmations or undo buttons is shown right at that moment
  3. Panic

Expected behavior

An undo button or a reset confirmation should be in visible area at that moment, right after clicked the reset button.

Possible solution

I think replacing reset button to 'Undo' would be great, like clicking

[[ Reset all ]]

is replaced to

✔️ All are reset [[ Undo ]]

.

Or putting the toast message at the bottom is also okay I think?

@rugk
Copy link
Owner

rugk commented Aug 15, 2023

That is a great idea and indeed a UX flaw. As you noticed, it needs to (or rather should) be implemented in https://github.com/TinyWebEx/AutomaticSettings/

Contributions are very welcome. I've also created an issue there to track it with more details of what I'd do implementation-wise: TinyWebEx/AutomaticSettings#28

@rugk rugk added the good first issue Good for newcomers label Aug 15, 2023
@renji18
Copy link

renji18 commented Aug 18, 2023

Hey @rugk, I'd like to work on this issue as I start my contribution into open source.

@rugk
Copy link
Owner

rugk commented Aug 19, 2023

Okay, great, please see what I've wrote before, that should give you the correct direction.

renji18 added a commit to renji18/offline-qr-code that referenced this issue Aug 19, 2023
renji18 added a commit to renji18/offline-qr-code that referenced this issue Aug 19, 2023
@TImmykiller5
Copy link

Can I still work on this?

@renji18 renji18 removed their assignment Aug 25, 2023
@rugk
Copy link
Owner

rugk commented Aug 29, 2023

As I saw @renji18 removed their assignment, I take this as a you for you @TImmykiller5. Anyway, feel free to work together/communicate here if something needs to be done etc.

So yeah, sure, you can take it. And thanks a lot in advance. If you have any issues or questions, e.g. how to implement a certain thing or how a thing works, feel free to ask. Also note GitHub's help can help on topics on how to use GitHub and e.g. how to create and update pull requests. Please also read our contributing guide to see how to get started with development and help us. And please read the past communication in this issue ans PRs to see what still needs to be done.

@Hamjaster
Copy link

I see this issue is still open, I'd like to work on it, I'd place the UNDO button aside reset button, and make it look good

Thanks in advance :)

@rugk
Copy link
Owner

rugk commented Sep 23, 2023

Okay, there is already a PR at #312, please have a look at it if it works out.
I assume, if help is needed, @renji18 you can jump in and you may work together. Please note as you can see there, this should be solved for all extensions using the https://github.com/TinyWebEx libraries.

@rugk rugk assigned Hamjaster and unassigned TImmykiller5 Sep 23, 2023
@Hamjaster
Copy link

I don't understand what you mean by " it should work on all extensions"??
And the github repo link you're giving?

Isnt it just a small css layout fix?

@xnuk
Copy link
Author

xnuk commented Sep 23, 2023

@Hamjaster As you can see you're now the 3rd assignee of this issue - previously two person stucked for resolving this. Personally I don't think this is a good first issue label though.

Isnt it just a small css layout fix?

This would be fixed by small javascript and css, if this project has simple enough structure. My first saw was, it isn't. The UI I referred in issue description depends on TinyWebEx libraries and it's little bit hard-coded - non-configurable for solving this issue. So... good luck?

@rugk
Copy link
Owner

rugk commented Sep 23, 2023

Personally I don't think this is a good first issue label though.

Oh yeah, nowadays, I agree, let's better remove it. It is indeed somewhat difficult.

The UI I referred in issue description depends on TinyWebEx libraries and it's little bit hard-coded

I am always open for improvements, at best as PRs.

@rugk rugk added help wanted Extra attention is needed and removed good first issue Good for newcomers labels Sep 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed usability/UX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants