ackzell / camati

Voicemail app to leave short notes and upload to a server.
https://camati.netlify.app/
1 stars 1 forks source link

added a confirm message to the remove recording method #38

Open simonjcarr opened 4 years ago

simonjcarr commented 4 years ago

Added a confirm dialog message before a recording is deleted.

netlify[bot] commented 4 years ago

‼️ Deploy request for camati rejected. Learn more about Netlify's sensitive variable policy

🔨 Explore the source changes: eaa98aa2321b7b8d0bac85e5c7e9d3f231f63a38

simonjcarr commented 4 years ago

I will take a look and submit a PR.

On Sun, 2 Aug 2020 at 17:55, Axel Uriel Martínez Castillo < notifications@github.com> wrote:

@ackzell requested changes on this pull request.

Thank you! I added a comment.

In components/RecordingsList.vue https://github.com/ackzell/camati/pull/38#discussion_r464099419:

  • if (!confirm('Are you sure you want to remove this record?')) {
  • return
  • }

Thanks for your contribution @simonjcarr https://github.com/simonjcarr! Much appreciated :)

We have an issue (#5 https://github.com/ackzell/camati/issues/5 ) related to this. I will update it to describe better the approach I would like to take.

Vuetify has a dialog component https://vuetifyjs.com/en/components/dialogs/#modal that we can use in this case.

Wanna use that instead?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ackzell/camati/pull/38#pullrequestreview-459663333, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALH6TSIHVRGICLIST6GHK3R6WK7FANCNFSM4PSQNDRA .

ackzell commented 4 years ago

Thanks a lot for your contribution @simonjcarr!

I have some feedback still 😬

In the image you can see the dialog grows as wide as there is space available in the window. Can we make that not the case and constrain the width?

As for the buttons, I was thinking to align them to the right and change the colors to be:

If possible, maybe some space around them to not be too close to the edges of the dialog. (I glanced on the docs and I think that is the default spacing, so that would be okay to remain the same).

Pasted_Image_8_10_20__8_39_PM

simonjcarr commented 4 years ago

I will have another look as soon as I can, I have some priorities at work at the moment and have recently started writing on Medium, which I am really enjoying but that is also taking my time at the moment.

I will also have to see if I can get my dev environment tied into Google for uploading the recordings, which is the reason why I was not able to test before I submitted the last PR. When time allows I will see if I can get this working, so I can visualize the results before submitting another PR. Feel free to make any changes yourself, I won't be offended.

Thanks and take care Simon

On Tue, 11 Aug 2020 at 02:48, Axel Uriel Martínez Castillo < notifications@github.com> wrote:

Thanks a lot for your contribution @simonjcarr https://github.com/simonjcarr!

I have some feedback still 😬

In the image you can see the dialog grows as wide as there is space available in the window. Can we make that not the case and constrain the width?

As for the buttons, I was thinking to align them to the right and change the colors to be:

  • Cancel: Secondary
  • Delete: Primary

If possible, maybe some space around them to not be too close to the edges of the dialog. (I glanced on the docs and I think that is the default spacing, so that would be okay to remain the same).

[image: Pasted_Image_8_10_20__8_39_PM] https://user-images.githubusercontent.com/601266/89847529-e1772980-db49-11ea-9664-a73848c603f2.png

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/ackzell/camati/pull/38#issuecomment-671676701, or unsubscribe https://github.com/notifications/unsubscribe-auth/AALH6TUNLIVIOQVZON53Q63SACPO7ANCNFSM4PSQNDRA .