Skip to content

Hard overwrite on conflict for query owners (re #283)#327

Merged
jezdez merged 2 commits into
masterfrom
hard-overwrite-283
Mar 5, 2018
Merged

Hard overwrite on conflict for query owners (re #283)#327
jezdez merged 2 commits into
masterfrom
hard-overwrite-283

Conversation

@washort

@washort washort commented Jan 30, 2018

Copy link
Copy Markdown

This fixes #283 though I'd love to see a better way to handle an action in a toastr notification than setting a global function as callback.

@jezdez

jezdez commented Feb 28, 2018

Copy link
Copy Markdown

@washort So with the looming issue of staying close with upstream in terms of API usage I just looked again at the toastr calls there. There are no uses of HTML in toastr popups and I think that means we should stay away from that as well.

In stead I think we can safely use the window.confirm function instead as it's used in various places in the code.

And since our confirmation query only applies to query owners it's okay UX wise.

In other words, depending on $scope.isQueryOwner I think we should just conditionally call window.confirm or toastr.error.

@jezdez

jezdez commented Feb 28, 2018

Copy link
Copy Markdown

@washort Do you want me to take a stab at this? (I know you've spent lots of time on this already)

@washort

washort commented Feb 28, 2018

Copy link
Copy Markdown
Author

Sure, if you don't mind having a look.

@jezdez
jezdez force-pushed the hard-overwrite-283 branch from c2cf29e to 6508675 Compare February 28, 2018 22:58
@jezdez

jezdez commented Feb 28, 2018

Copy link
Copy Markdown

@washort Okay, found an own AlertDialog thing actually, which is what is used in other places already that fits the bill better than window.confirm.

@jezdez
jezdez force-pushed the hard-overwrite-283 branch from 6508675 to 5079ca2 Compare February 28, 2018 23:01
@washort

washort commented Mar 1, 2018

Copy link
Copy Markdown
Author

r+, this is obviously a better solution

@jezdez
jezdez merged this pull request into master Mar 5, 2018
@jezdez
jezdez deleted the hard-overwrite-283 branch March 5, 2018 16:28
@jezdez

jezdez commented Jan 25, 2019

Copy link
Copy Markdown

This got merged upstream as well.

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.

Owners of queries should be able to hard overwrite

2 participants