Simplify commit and snippet notes
What does this MR do and why?
- Fixes vertical line
- Removes extra system note for threads (and collapse functionality)
- Why? We don't actually show the
Reply
option here anyway; you can only get to this state if you selectStart a thread
instead ofComment
. And the collapsed state isn't persisted.
- Why? We don't actually show the
- Fix mobile styles
- On mobile, hides
Edit
icon button and addsEdit comment
option in actions dropdown
What does this MR not do?
- Convert the notes to Vue
Screenshots or screen recordings
Before | After |
---|---|
![]() |
![]() |
![]() |
![]() |
How to set up and validate locally
Check areas that use our legacy notes:
- Snippet
- Commit
- Are there any others? I thought wiki used them too but I don't actually see a way to comment anywhere on wikis
🤔
Links
Merge request reports
Activity
changed milestone to %16.11
added UX UX Paper Cuts frontend maintenanceusability typemaintenance labels
assigned to @annabeldunstone
added 1 commit
- 9cbc2b84 - Simplify HAML notes in commits/snippets/wiki
Todo:
- Badge spacing
- Mobile
Edited by Annabel Dunstone Grayadded Category:Source Code Management label
added devopscreate groupsource code sectiondev labels
- A deleted user
added backend label
5 Warnings ⚠ 2d6bb916: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. ⚠ 763b6217: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. ⚠ 41d20d0b: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. ⚠ 6e6b7f44: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. ⚠ This merge request contains lines with testid selectors. Please ensure e2e:package-and-test
job is run.1 Message 📖 CHANGELOG missing: If this merge request needs a changelog entry, add the
Changelog
trailer to the commit message you want to add to the changelog.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
testid
selectorsThe following changed lines in this MR contain
testid
selectors:app/views/projects/notes/_actions.html.haml
- button_options: { class: 'note-action-button js-note-edit has-tooltip', data: { container: 'body', testid: 'edit-comment-button' }, title: _('Edit comment'), 'aria-label': _('Edit comment') }) + button_options: { class: 'gl-display-none gl-sm-display-inline-block note-action-button js-note-edit has-tooltip', data: { testid: 'edit-comment-button' }, title: _('Edit comment'), 'aria-label': _('Edit comment') })
app/views/projects/notes/_more_actions_dropdown.html.haml
- = render Pajamas::ButtonComponent.new(icon: 'ellipsis_v', category: :tertiary, button_options: { class: 'note-action-button more-actions-toggle has-tooltip', data: { title: 'More actions', toggle: 'dropdown', container: 'body', testid: 'more-actions-dropdown' }}) + = render Pajamas::ButtonComponent.new(icon: 'ellipsis_v', category: :tertiary, button_options: { class: 'note-action-button more-actions-toggle', data: { title: 'More actions', toggle: 'dropdown', container: 'body', testid: 'more-actions-dropdown' }}) + = render Pajamas::ButtonComponent.new(category: :tertiary, button_options: { class: 'menu-item note-action-button js-note-edit', data: { container: 'body', testid: 'edit-comment-button' } }) do
app/views/shared/notes/_note.html.haml
- %spannote-headline-light{ data: { testid: 'note-author-content' } } + %span.note-headline-light{ data: { testid: 'note-author-content' } } - .note-text.md{ data: { testid: 'note-content' } } + .note-text.md{ data: { testid: 'note-content' } }
app/views/snippets/notes/_actions.html.haml
- = render Pajamas::ButtonComponent.new(category: :tertiary, icon: 'pencil', button_options: { title: _('Edit comment'), class: 'note-action-button js-note-edit has-tooltip', data: { container: 'body', testid: 'edit-comment-button' } }) + = render Pajamas::ButtonComponent.new(category: :tertiary, icon: 'pencil', button_options: { title: _('Edit comment'), class: 'note-action-button js-note-edit has-tooltip gl-display-none gl-sm-display-block', data: { container: 'body', testid: 'edit-comment-button' } })
If the
e2e:package-and-test
job in theqa
stage has run automatically, please ensure the tests are passing. If the job has not run, please start themanual:e2e-test-pipeline-generate
job in theprepare
stage and ensure the tests infollow-up:e2e:package-and-test-ee
pipeline are passing.For the list of known failures please refer to the latest pipeline triage issue.
If your changes are under a feature flag, please check our Testing with feature flags documentation for instructions.
Reviewer roulette
Category Reviewer Maintainer backend @j_lar
(UTC+2, 2 hours ahead of author)
@ayufan
(UTC+2, 2 hours ahead of author)
frontend @thomasrandolph
(UTC-6, 6 hours behind author)
@pgascouvaillancourt
(UTC-4, 4 hours behind author)
Please check reviewer's status!
Please refer to documentation page for guidance on how you can benefit from the Reviewer Roulette, or use the GitLab Review Workload Dashboard to find other available reviewers.
If needed, you can retry the
🔁 danger-review
job that generated this comment.Generated by
🚫 Dangeradded pipeline:skip-pajamas-adoption label
- Resolved by Annabel Dunstone Gray
Adding pipeline:skip-pajamas-adoption because this is legacy code that will eventually be completely removed in favor of the Vue notes we're using in MRs/issues/etc
🤞
added 1 commit
- 27e68aa8 - Remove unnecessary classes and css; fix badges
Bundle size analysis [beta]
This compares changes in bundle size for entry points between the commits 05f6ebfe and 2d6bb916
✨ Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 4.32 MB 4.32 MB - 0.0 % mainChunk 3.31 MB 3.31 MB - 0.0 %
Please look at the full report for more details
Read more about how this report works.
Generated by
🚫 Danger- Resolved by Annabel Dunstone Gray
@clavimoniere Can you please do an initial UX review here? This should only affect snippets, commits, and wiki comments as they're still using our legacy notes (in haml)
requested review from @clavimoniere
removed review request for @clavimoniere
added pipeline:mr-approved label
- Resolved by Annabel Dunstone Gray
👋 @clavimoniere
, thanks for approving this merge request.This is the first time the merge request has been approved. To ensure we don't only run predictive pipelines, and we don't break
master
, a new pipeline will be started shortly.Please wait for the pipeline to start before resolving this discussion and set auto-merge for the new pipeline. See merging a merge request for more details.
E2E Test Result Summary
allure-report-publisher
generated test report!e2e-test-on-gdk:
✅ test report for 2d6bb916expand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Govern | 66 | 0 | 0 | 0 | 66 | ✅ | | Create | 83 | 0 | 9 | 0 | 92 | ✅ | | Plan | 51 | 0 | 2 | 0 | 53 | ✅ | | Package | 24 | 0 | 6 | 0 | 30 | ✅ | | Data Stores | 31 | 0 | 0 | 0 | 31 | ✅ | | Monitor | 7 | 0 | 0 | 0 | 7 | ✅ | | Verify | 35 | 0 | 1 | 0 | 36 | ✅ | | Analytics | 2 | 0 | 0 | 0 | 2 | ✅ | | Release | 5 | 0 | 0 | 0 | 5 | ✅ | | Manage | 0 | 0 | 1 | 0 | 1 | ➖ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 304 | 0 | 19 | 0 | 323 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
e2e-package-and-test:
✅ test report for 2d6bb916expand test summary
+------------------------------------------------------------------+ | suites summary | +-------------+--------+--------+---------+-------+-------+--------+ | | passed | failed | skipped | flaky | total | result | +-------------+--------+--------+---------+-------+-------+--------+ | Create | 570 | 0 | 66 | 6 | 636 | ✅ | | Package | 0 | 0 | 2 | 0 | 2 | ➖ | | Plan | 8 | 0 | 0 | 0 | 8 | ✅ | | Govern | 6 | 0 | 0 | 0 | 6 | ✅ | | Monitor | 8 | 0 | 0 | 0 | 8 | ✅ | | Data Stores | 4 | 0 | 0 | 0 | 4 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+ | Total | 596 | 0 | 68 | 6 | 664 | ✅ | +-------------+--------+--------+---------+-------+-------+--------+
added 106 commits
-
dad10c5b...67a0be09 - 102 commits from branch
master
- fcc356f4 - Simplify HAML notes in commits/snippets/wiki
- a95458dd - Remove unnecessary classes and css; fix badges
- 596f7c65 - Fix mobile styles
- d8f09894 - Fix specs; remove potentially unneeded stuff
Toggle commit list-
dad10c5b...67a0be09 - 102 commits from branch
requested review from @mle
- Resolved by Annabel Dunstone Gray
- Resolved by Phil Hughes
@rcrespo3 Can you please review frontend here?
@sam.figueroa Can you please review the backend changes? I'm wondering if there's more that could be removed here
🤔
requested review from @rcrespo3 and @sam.figueroa
mentioned in issue gitlab-com/www-gitlab-com#34689 (closed)
- Resolved by Annabel Dunstone Gray
- Resolved by Annabel Dunstone Gray
added 232 commits
-
d8f09894...2c999496 - 228 commits from branch
master
- abf3891c - Simplify HAML notes in commits/snippets/wiki
- 549eee21 - Remove unnecessary classes and css; fix badges
- 9616ab28 - Fix mobile styles
- 878176fc - Fix specs; remove potentially unneeded stuff
Toggle commit list-
d8f09894...2c999496 - 228 commits from branch
added 1 commit
- d6bf40a5 - Fix specs; remove potentially unneeded stuff
- Resolved by Rudy Crespo
requested review from @rcrespo3
requested review from @rcrespo3
added 46 commits
-
d6bf40a5...02709140 - 42 commits from branch
master
- f02aff30 - Simplify HAML notes in commits/snippets/wiki
- 0def7f41 - Remove unnecessary classes and css; fix badges
- 2142ff4a - Fix mobile styles
- 83a51564 - Fix specs; remove potentially unneeded stuff
Toggle commit list-
d6bf40a5...02709140 - 42 commits from branch
- Resolved by Annabel Dunstone Gray
removed review request for @mle
- Resolved by Annabel Dunstone Gray