Skip to content
Projects
Groups
Snippets
Help
Loading...
Help
Support
Keyboard shortcuts
?
Submit feedback
Contribute to GitLab
Sign in / Register
Toggle navigation
G
gitlab-ce
Project overview
Project overview
Details
Activity
Releases
Repository
Repository
Files
Commits
Branches
Tags
Contributors
Graph
Compare
Issues
0
Issues
0
List
Boards
Labels
Milestones
Merge Requests
1
Merge Requests
1
Analytics
Analytics
Repository
Value Stream
Wiki
Wiki
Snippets
Snippets
Members
Members
Collapse sidebar
Close sidebar
Activity
Graph
Create a new issue
Commits
Issue Boards
Open sidebar
nexedi
gitlab-ce
Commits
ee280ed2
Commit
ee280ed2
authored
Nov 16, 2021
by
Olena Horal-Koretska
Committed by
Peter Hegman
Nov 16, 2021
Browse files
Options
Browse Files
Download
Email Patches
Plain Diff
Replace `window.confirm` with `GlModal` confirmation
When canceling a MR comment Changelog: changed
parent
ec60abcf
Changes
4
Show whitespace changes
Inline
Side-by-side
Showing
4 changed files
with
70 additions
and
44 deletions
+70
-44
app/assets/javascripts/diffs/components/diff_line_note_form.vue
...sets/javascripts/diffs/components/diff_line_note_form.vue
+5
-4
app/assets/javascripts/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal.js
...ts/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal.js
+26
-18
spec/features/merge_request/user_posts_diff_notes_spec.rb
spec/features/merge_request/user_posts_diff_notes_spec.rb
+4
-2
spec/frontend/diffs/components/diff_line_note_form_spec.js
spec/frontend/diffs/components/diff_line_note_form_spec.js
+35
-20
No files found.
app/assets/javascripts/diffs/components/diff_line_note_form.vue
View file @
ee280ed2
...
...
@@ -2,6 +2,7 @@
import
{
mapState
,
mapGetters
,
mapActions
}
from
'
vuex
'
;
import
{
s__
}
from
'
~/locale
'
;
import
diffLineNoteFormMixin
from
'
~/notes/mixins/diff_line_note_form
'
;
import
{
confirmAction
}
from
'
~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal
'
;
import
glFeatureFlagsMixin
from
'
~/vue_shared/mixins/gl_feature_flags_mixin
'
;
import
MultilineCommentForm
from
'
../../notes/components/multiline_comment_form.vue
'
;
import
{
...
...
@@ -177,16 +178,16 @@ export default {
'
saveDiffDiscussion
'
,
'
setSuggestPopoverDismissed
'
,
]),
handleCancelCommentForm
(
shouldConfirm
,
isDirty
)
{
async
handleCancelCommentForm
(
shouldConfirm
,
isDirty
)
{
if
(
shouldConfirm
&&
isDirty
)
{
const
msg
=
s__
(
'
Notes|Are you sure you want to cancel creating this comment?
'
);
// eslint-disable-next-line no-alert
if
(
!
window
.
confirm
(
msg
))
{
const
confirmed
=
await
confirmAction
(
msg
);
if
(
!
confirmed
)
{
return
;
}
}
this
.
cancelCommentForm
({
lineCode
:
this
.
line
.
line_code
,
fileHash
:
this
.
diffFileHash
,
...
...
app/assets/javascripts/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal.js
View file @
ee280ed2
import
Vue
from
'
vue
'
;
export
function
confirm
ViaGlModal
(
message
,
element
)
{
export
function
confirm
Action
(
message
,
{
primaryBtnVariant
,
primaryBtnText
}
=
{}
)
{
return
new
Promise
((
resolve
)
=>
{
let
confirmed
=
false
;
const
props
=
{};
const
confirmBtnVariant
=
element
.
getAttribute
(
'
data-confirm-btn-variant
'
);
if
(
confirmBtnVariant
)
{
props
.
primaryVariant
=
confirmBtnVariant
;
}
const
screenReaderText
=
element
.
querySelector
(
'
.gl-sr-only
'
)?.
textContent
||
element
.
querySelector
(
'
.sr-only
'
)?.
textContent
||
element
.
getAttribute
(
'
aria-label
'
);
if
(
screenReaderText
)
{
props
.
primaryText
=
screenReaderText
;
}
const
component
=
new
Vue
({
components
:
{
ConfirmModal
:
()
=>
import
(
'
./confirm_modal.vue
'
),
...
...
@@ -28,7 +12,10 @@ export function confirmViaGlModal(message, element) {
return
h
(
'
confirm-modal
'
,
{
props
,
props
:
{
primaryVariant
:
primaryBtnVariant
,
primaryText
:
primaryBtnText
,
},
on
:
{
confirmed
()
{
confirmed
=
true
;
...
...
@@ -45,3 +32,24 @@ export function confirmViaGlModal(message, element) {
}).
$mount
();
});
}
export
function
confirmViaGlModal
(
message
,
element
)
{
const
primaryBtnConfig
=
{};
const
confirmBtnVariant
=
element
.
getAttribute
(
'
data-confirm-btn-variant
'
);
if
(
confirmBtnVariant
)
{
primaryBtnConfig
.
primaryBtnVariant
=
confirmBtnVariant
;
}
const
screenReaderText
=
element
.
querySelector
(
'
.gl-sr-only
'
)?.
textContent
||
element
.
querySelector
(
'
.sr-only
'
)?.
textContent
||
element
.
getAttribute
(
'
aria-label
'
);
if
(
screenReaderText
)
{
primaryBtnConfig
.
primaryBtnText
=
screenReaderText
;
}
return
confirmAction
(
message
,
primaryBtnConfig
);
}
spec/features/merge_request/user_posts_diff_notes_spec.rb
View file @
ee280ed2
...
...
@@ -238,8 +238,10 @@ RSpec.describe 'Merge request > User posts diff notes', :js do
def
should_allow_dismissing_a_comment
(
line_holder
,
diff_side
=
nil
)
write_comment_on_line
(
line_holder
,
diff_side
)
accept_confirm
do
find
(
'.js-close-discussion-note-form'
).
click
page
.
within
(
'.modal'
)
do
click_button
'OK'
end
assert_comment_dismissal
(
line_holder
)
...
...
spec/frontend/diffs/components/diff_line_note_form_spec.js
View file @
ee280ed2
import
{
shallowMount
}
from
'
@vue/test-utils
'
;
import
{
nextTick
}
from
'
vue
'
;
import
DiffLineNoteForm
from
'
~/diffs/components/diff_line_note_form.vue
'
;
import
{
createStore
}
from
'
~/mr_notes/stores
'
;
import
NoteForm
from
'
~/notes/components/note_form.vue
'
;
import
{
confirmAction
}
from
'
~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal
'
;
import
{
noteableDataMock
}
from
'
../../notes/mock_data
'
;
import
diffFileMockData
from
'
../mock_data/diff_file
'
;
jest
.
mock
(
'
~/lib/utils/confirm_via_gl_modal/confirm_via_gl_modal
'
,
()
=>
{
return
{
confirmAction
:
jest
.
fn
(),
};
});
describe
(
'
DiffLineNoteForm
'
,
()
=>
{
let
wrapper
;
let
diffFile
;
...
...
@@ -36,49 +44,56 @@ describe('DiffLineNoteForm', () => {
});
};
const
findNoteForm
=
()
=>
wrapper
.
findComponent
(
NoteForm
);
describe
(
'
methods
'
,
()
=>
{
beforeEach
(()
=>
{
wrapper
=
createComponent
();
});
describe
(
'
handleCancelCommentForm
'
,
()
=>
{
afterEach
(()
=>
{
confirmAction
.
mockReset
();
});
it
(
'
should ask for confirmation when shouldConfirm and isDirty passed as truthy
'
,
()
=>
{
jest
.
spyOn
(
window
,
'
confirm
'
).
mockReturnValu
e
(
false
);
confirmAction
.
mockResolvedValueOnc
e
(
false
);
wrapper
.
vm
.
handleCancelCommentForm
(
true
,
true
);
findNoteForm
().
vm
.
$emit
(
'
cancelForm
'
,
true
,
true
);
expect
(
window
.
confirm
).
toHaveBeenCalled
();
expect
(
confirmAction
).
toHaveBeenCalled
();
});
it
(
'
should ask for confirmation when one of the params false
'
,
()
=>
{
jest
.
spyOn
(
window
,
'
confirm
'
).
mockReturnValu
e
(
false
);
it
(
'
should
not
ask for confirmation when one of the params false
'
,
()
=>
{
confirmAction
.
mockResolvedValueOnc
e
(
false
);
wrapper
.
vm
.
handleCancelCommentForm
(
true
,
false
);
findNoteForm
().
vm
.
$emit
(
'
cancelForm
'
,
true
,
false
);
expect
(
window
.
confirm
).
not
.
toHaveBeenCalled
();
expect
(
confirmAction
).
not
.
toHaveBeenCalled
();
wrapper
.
vm
.
handleCancelCommentForm
(
false
,
true
);
findNoteForm
().
vm
.
$emit
(
'
cancelForm
'
,
false
,
true
);
expect
(
window
.
confirm
).
not
.
toHaveBeenCalled
();
expect
(
confirmAction
).
not
.
toHaveBeenCalled
();
});
it
(
'
should call cancelCommentForm with lineCode
'
,
(
done
)
=>
{
jest
.
spyOn
(
window
,
'
confirm
'
).
mockImplementation
(()
=>
{}
);
it
(
'
should call cancelCommentForm with lineCode
'
,
async
(
)
=>
{
confirmAction
.
mockResolvedValueOnce
(
true
);
jest
.
spyOn
(
wrapper
.
vm
,
'
cancelCommentForm
'
).
mockImplementation
(()
=>
{});
jest
.
spyOn
(
wrapper
.
vm
,
'
resetAutoSave
'
).
mockImplementation
(()
=>
{});
wrapper
.
vm
.
handleCancelCommentForm
();
expect
(
window
.
confirm
).
not
.
toHaveBeenCalled
();
wrapper
.
vm
.
$nextTick
(()
=>
{
findNoteForm
().
vm
.
$emit
(
'
cancelForm
'
,
true
,
true
);
await
nextTick
();
expect
(
confirmAction
).
toHaveBeenCalled
();
await
nextTick
();
expect
(
wrapper
.
vm
.
cancelCommentForm
).
toHaveBeenCalledWith
({
lineCode
:
diffLines
[
1
].
line_code
,
fileHash
:
wrapper
.
vm
.
diffFileHash
,
});
expect
(
wrapper
.
vm
.
resetAutoSave
).
toHaveBeenCalled
();
done
();
});
});
});
...
...
Write
Preview
Markdown
is supported
0%
Try again
or
attach a new file
Attach a file
Cancel
You are about to add
0
people
to the discussion. Proceed with caution.
Finish editing this message first!
Cancel
Please
register
or
sign in
to comment