Skip to content

Conversation

@magicznyleszek
Copy link
Member

@magicznyleszek magicznyleszek commented Sep 27, 2018

Description

Works on all: Escape key, backdrop click and "X" button click. Code allows for adding same functionality in other modals.

Related issues

Fixes #2006
Fixes #2011

@jnm
Copy link
Member

jnm commented Sep 27, 2018

A few observations:

  • The "BACK" button doesn't trigger the warning, and changes are lost.
  • (minor gripe) "Close Translations table" has funky capitalization.

I had some earlier comments that I've since erased about the app crashing with a white screen, but that problem is not related to this PR. It's now tracked at #2011.

@magicznyleszek
Copy link
Member Author

magicznyleszek commented Sep 28, 2018

@jnm I'v added confirm for BACK button and updated some text labels.

I also found an easy way to know if there are unsaved changes and only apply BACK confirm then.

Do you think it's worthwhile to apply this logic to modal's close/backdrop click? It would be a bit more complex, as the code for closing modal is completely separated from modal's content code.

Also: I fixed the error from #2011

@jnm
Copy link
Member

jnm commented Oct 2, 2018

@magicznyleszek, I think it's worthwhile but less urgent. Could you create an issue for that and prioritize it as you see fit?

@jnm jnm merged commit ad048dc into master Oct 2, 2018
@jnm jnm deleted the issue-2006 branch October 2, 2018 18:18
@jnm jnm mentioned this pull request Oct 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants