From 438561bb95bd7b529f2db6856ce27935dc92e2a4 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Tue, 25 Feb 2025 20:50:57 -0700 Subject: [PATCH 1/3] Add button for the diff file header options menu [Rapid Diffs] --- app/components/rapid_diffs/diff_file_header_component.html.haml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/app/components/rapid_diffs/diff_file_header_component.html.haml b/app/components/rapid_diffs/diff_file_header_component.html.haml index 22e81aa36aa073..15c920c223591b 100644 --- a/app/components/rapid_diffs/diff_file_header_component.html.haml +++ b/app/components/rapid_diffs/diff_file_header_component.html.haml @@ -48,3 +48,5 @@ %span.rd-lines-removed %span>= "−".html_safe %span{ "data-testid" => "js-file-deletion-line" }= @diff_file.removed_lines + .rd-diff-file-options-menu.gl-ml-2 + = render Pajamas::ButtonComponent.new(category: :tertiary, size: :small, icon: 'ellipsis_v', button_options: { data: { click: 'toggleOptionsMenu' }, aria: { label: _('Options') } }) -- GitLab From 23cc427aef8fd5303b6034a51d735c78bd25b509 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 26 Feb 2025 11:12:14 -0700 Subject: [PATCH 2/3] Add a Vue dropdown when the user clicks the options menu --- .../javascripts/rapid_diffs/adapters.js | 3 +- .../rapid_diffs/options_menu/adapter.js | 28 +++++++++ .../diff_file_header_component.html.haml | 3 +- .../rapid_diffs/options_menu/adapter_spec.js | 63 +++++++++++++++++++ 4 files changed, 95 insertions(+), 2 deletions(-) create mode 100644 app/assets/javascripts/rapid_diffs/options_menu/adapter.js create mode 100644 spec/frontend/rapid_diffs/options_menu/adapter_spec.js diff --git a/app/assets/javascripts/rapid_diffs/adapters.js b/app/assets/javascripts/rapid_diffs/adapters.js index fb9c404263e149..d5c5691c0282c4 100644 --- a/app/assets/javascripts/rapid_diffs/adapters.js +++ b/app/assets/javascripts/rapid_diffs/adapters.js @@ -1,4 +1,5 @@ import { ExpandLinesAdapter } from '~/rapid_diffs/expand_lines/adapter'; +import { OptionsMenuAdapter } from '~/rapid_diffs/options_menu/adapter'; import { ToggleFileAdapter } from '~/rapid_diffs/toggle_file/adapter'; const RAPID_DIFFS_VIEWERS = { @@ -6,7 +7,7 @@ const RAPID_DIFFS_VIEWERS = { text_parallel: 'text_parallel', }; -const COMMON_ADAPTERS = [ExpandLinesAdapter, ToggleFileAdapter]; +const COMMON_ADAPTERS = [ExpandLinesAdapter, OptionsMenuAdapter, ToggleFileAdapter]; export const VIEWER_ADAPTERS = { [RAPID_DIFFS_VIEWERS.text_inline]: COMMON_ADAPTERS, diff --git a/app/assets/javascripts/rapid_diffs/options_menu/adapter.js b/app/assets/javascripts/rapid_diffs/options_menu/adapter.js new file mode 100644 index 00000000000000..80cf7269b82452 --- /dev/null +++ b/app/assets/javascripts/rapid_diffs/options_menu/adapter.js @@ -0,0 +1,28 @@ +import Vue from 'vue'; +import { GlDisclosureDropdown } from '@gitlab/ui'; + +export const OptionsMenuAdapter = { + clicks: { + toggleOptionsMenu(event) { + const button = event.target.closest('.js-options-button'); + + if (!this.sink.optionsMenu) { + this.sink.optionsMenu = new Vue({ + el: button, + name: 'GlDisclosureDropdown', + render: (createElement) => + createElement(GlDisclosureDropdown, { + props: { + icon: 'ellipsis_v', + startOpened: true, + noCaret: true, + category: 'tertiary', + size: 'small', + toggleClass: 'js-options-disclosure', + }, + }), + }); + } + }, + }, +}; diff --git a/app/components/rapid_diffs/diff_file_header_component.html.haml b/app/components/rapid_diffs/diff_file_header_component.html.haml index 15c920c223591b..02b5ef07ab2784 100644 --- a/app/components/rapid_diffs/diff_file_header_component.html.haml +++ b/app/components/rapid_diffs/diff_file_header_component.html.haml @@ -49,4 +49,5 @@ %span>= "−".html_safe %span{ "data-testid" => "js-file-deletion-line" }= @diff_file.removed_lines .rd-diff-file-options-menu.gl-ml-2 - = render Pajamas::ButtonComponent.new(category: :tertiary, size: :small, icon: 'ellipsis_v', button_options: { data: { click: 'toggleOptionsMenu' }, aria: { label: _('Options') } }) + .js-options-menu + = render Pajamas::ButtonComponent.new(category: :tertiary, size: :small, icon: 'ellipsis_v', button_options: { class: 'js-options-button', data: { click: 'toggleOptionsMenu' }, aria: { label: _('Options') } }) diff --git a/spec/frontend/rapid_diffs/options_menu/adapter_spec.js b/spec/frontend/rapid_diffs/options_menu/adapter_spec.js new file mode 100644 index 00000000000000..8561f3c36e788b --- /dev/null +++ b/spec/frontend/rapid_diffs/options_menu/adapter_spec.js @@ -0,0 +1,63 @@ +import { DiffFile } from '~/rapid_diffs/diff_file'; +import { OptionsMenuAdapter } from '~/rapid_diffs/options_menu/adapter'; + +describe('Diff File Options Menu', () => { + const html = ` + +
+
+
+
+ +
+
+
+
+ + + `; + + function get(element) { + const elements = { + file: () => document.querySelector('diff-file'), + container: () => get('file').querySelector('.js-options-menu'), + serverButton: () => get('container').querySelector('.js-options-button'), + vueButton: () => get('container').querySelector('.js-options-disclosure'), + }; + + return elements[element]?.(); + } + + function assignAdapter(customAdapter) { + get('file').adapterConfig = { any: [customAdapter] }; + } + + beforeAll(() => { + customElements.define('diff-file', DiffFile); + }); + + beforeEach(() => { + document.body.innerHTML = html; + assignAdapter(OptionsMenuAdapter); + get('file').mount(); + }); + + it('starts with the server-rendered button', () => { + expect(get('serverButton')).not.toBeNull(); + }); + + it('replaces the server-rendered button with a Vue GlDisclosureDropdown when the button is clicked', () => { + const button = get('serverButton'); + + expect(get('vueButton')).toBeNull(); + expect(button).not.toBeNull(); + + button.click(); + + expect(get('vueButton')).not.toBeNull(); + /* This button being replaced also means this replacement can only + * happen once (desireable!), so testing that it's no longer present is good + */ + expect(get('serverButton')).toBeNull(); + }); +}); -- GitLab From 8d459aab3ff19efa036ae9e266f9388d7d61ec82 Mon Sep 17 00:00:00 2001 From: Thomas Randolph Date: Wed, 26 Feb 2025 22:42:40 -0700 Subject: [PATCH 3/3] Fix issues with Vue 3 - Using the passed renderer (`createElement`) in the `render` function fails in Vue 3 - The only fix here is to explicitly use `Vue.h` - However, using `Vue.h` directly fails in the actual app, so - in Vue 3 - when the renderer isn't passed, we fill it with the Vue 3 `Vue.h` - The Vue 3 renderer does not work the same way the Vue 2 one does - The 'el' is REPLACED in Vue2, and used as the PARENT in Vue3 - `toggleClass` is not applied to the DisclosureDropdown in Vue3 --- app/assets/javascripts/rapid_diffs/options_menu/adapter.js | 5 ++--- spec/frontend/rapid_diffs/options_menu/adapter_spec.js | 5 +++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/app/assets/javascripts/rapid_diffs/options_menu/adapter.js b/app/assets/javascripts/rapid_diffs/options_menu/adapter.js index 80cf7269b82452..e7cc7f9d2bc784 100644 --- a/app/assets/javascripts/rapid_diffs/options_menu/adapter.js +++ b/app/assets/javascripts/rapid_diffs/options_menu/adapter.js @@ -8,9 +8,9 @@ export const OptionsMenuAdapter = { if (!this.sink.optionsMenu) { this.sink.optionsMenu = new Vue({ - el: button, + el: Vue.version.startsWith('2') ? button : button.parentElement, name: 'GlDisclosureDropdown', - render: (createElement) => + render: (createElement = Vue.h) => createElement(GlDisclosureDropdown, { props: { icon: 'ellipsis_v', @@ -18,7 +18,6 @@ export const OptionsMenuAdapter = { noCaret: true, category: 'tertiary', size: 'small', - toggleClass: 'js-options-disclosure', }, }), }); diff --git a/spec/frontend/rapid_diffs/options_menu/adapter_spec.js b/spec/frontend/rapid_diffs/options_menu/adapter_spec.js index 8561f3c36e788b..27a9c790ce12fb 100644 --- a/spec/frontend/rapid_diffs/options_menu/adapter_spec.js +++ b/spec/frontend/rapid_diffs/options_menu/adapter_spec.js @@ -22,7 +22,7 @@ describe('Diff File Options Menu', () => { file: () => document.querySelector('diff-file'), container: () => get('file').querySelector('.js-options-menu'), serverButton: () => get('container').querySelector('.js-options-button'), - vueButton: () => get('container').querySelector('.js-options-disclosure'), + vueButton: () => get('container').querySelector('[data-testid="base-dropdown-toggle"]'), }; return elements[element]?.(); @@ -55,7 +55,8 @@ describe('Diff File Options Menu', () => { button.click(); expect(get('vueButton')).not.toBeNull(); - /* This button being replaced also means this replacement can only + /* + * This button being replaced also means this replacement can only * happen once (desireable!), so testing that it's no longer present is good */ expect(get('serverButton')).toBeNull(); -- GitLab