699: Fix issue with dismissing warnings would create a "duplicate" entry #717
Conversation
@@ -321,6 +322,7 @@ $gp.editor = ( | |||
success: function( data ) { | |||
$gp.notices.success( 'Saved!' ); | |||
$gp.editor.replace_current( data ); | |||
$gp.editor.next(); |
ocean90
May 30, 2017
Member
Jumping to the next editor is a bad idea because 1) a translation can have more than one warning (see #370) and 2) a translation can be in a pending state.
In both steps you'd have to re-open the previous editor for further status changes, that sounds like a step backward to me.
Jumping to the next editor is a bad idea because 1) a translation can have more than one warning (see #370) and 2) a translation can be in a pending state.
In both steps you'd have to re-open the previous editor for further status changes, that sounds like a step backward to me.
toolstack
May 31, 2017
Author
Contributor
There were two reasons I added the next()
, first was that it called the right logic to display things correctly and second was to keep consistent with how other actions in the editor work (any time you it save, cancel, set a status, etc. the editor moves to the next entry).
I can see though in this case that may not be the desired outcome so I've updated the PR and removed some logic that was based on the assumption that whenever replace_current()
was called that the preview would be visible (the old logic was a little redundant all things considered).
There were two reasons I added the next()
, first was that it called the right logic to display things correctly and second was to keep consistent with how other actions in the editor work (any time you it save, cancel, set a status, etc. the editor moves to the next entry).
I can see though in this case that may not be the desired outcome so I've updated the PR and removed some logic that was based on the assumption that whenever replace_current()
was called that the preview would be visible (the old logic was a little redundant all things considered).
b318505
to
79d9e39
Thanks @ocean90 and @toolstack |
toolstack commentedMay 10, 2017
Resolves #699.