HabitRPG / habitica

A habit tracker app which treats your goals like a Role Playing Game.
https://habitica.com
Other
12k stars 4.09k forks source link

Replace alert() and confirm() with modals #9211

Open SabreCat opened 7 years ago

SabreCat commented 7 years ago

Description

From our internal bug reporting doc:

Don’t use default browser alert() [toGHtech] [priority: important][design!]

There are still some places where we use an alert() or confirm(), producing an ugly default JavaScript popup. These should be replaced with modals.

Example alert(): https://github.com/HabitRPG/habitica/blob/develop/website/client/components/static/home.vue#L597 Example confirm(): https://github.com/HabitRPG/habitica/blob/develop/website/client/components/challenges/closeChallengeModal.vue#L128

(Note these are also hardcoded English strings instead of i18n; see #9210 !)

shivasheeshyadav commented 7 years ago

@SabreCat I would like to work on this issue. Please assign it to me.

Alys commented 7 years ago

@shivasheeshyadav Thank you! Comment here if you have questions or find problems.

SSKUltra commented 7 years ago

Hey @shivasheeshyadav, I have added some code that uses confirm() method. If you have already added code that fixes this issue with modals, it would be great if you could share the screen-caps/code so that I make similar changes to my PR and all confirmation modals look the same throughout the site?

lemoness commented 6 years ago

@shivasheeshyadav Hi there! Just checking in -- how is this fix going?

Alys commented 6 years ago

I'm putting this on hold until https://github.com/HabitRPG/habitica/issues/9986 is fixed (modals reopening themselves) because we don't need more modals while that's happening. @shivasheeshyadav When it comes off hold it will be marked as "help wanted" but if you're still interested in working on it then you can reclaim it again.

Alys commented 6 years ago

I've taken this off hold.

Neumand commented 5 years ago

I'd like to work on this!

Alys commented 5 years ago

@Neumand Thank you! Please go ahead!

There might be quite a few places where alert() and confirm() are used so if you want to submit a pull request for just some of them, I think that would be fine. However if Habitica's staff comment here to say differently, listen to them not me. :)

shanaqui commented 4 years ago

Hi @Neumand, are you still interested in working on this? Please let us know! :)

shanaqui commented 4 years ago

I'm marking this as help wanted again. :)