search

briqNFT / briq-builder

DApp to build with briqs
21 stars 12 forks source link

feat(modals): added modal to confirm clear all #21

Closed fberger-xyz closed 2 years ago

fberger-xyz commented 2 years ago

Hi!

Context: https://discord.com/channels/906209682396413972/918903331232043079/967546514681524294

I was enjoying the edit of a set and miscliked on the Clear All. button. image

Unfortunately I could not manage to undo that misclick. => So I thought a confirmation modal would be useful.

image

🧱 🧱 🧱 🧱

wraitii commented 2 years ago

Hey ! I missed your PR, sorry.

Code looks generally good but I'd rather not add a modal for that since undo/redo is supposed to work for clear (and seems to when testing locally), and my general rule of thumb has been "undo/redo over modals".

Is it something you're maybe able to reproduce?

(edit -> also I pushed a linter, so you'll need to rebase)

fberger-xyz commented 2 years ago

Hi @wraitii !!

ok I understand! thanks for the explanation :) was a bit busy this week so thank you for giving me the time to answer

could not reproduce the bug although I have used your dApp a lot recently 🔥 I linted the code just to make the PR good looking but I will close it after submitting this comment