From a28ec3786871caf025d960f1319e061ec2c5cff2 Mon Sep 17 00:00:00 2001 From: ulferts Date: Thu, 19 Dec 2024 09:47:22 +0100 Subject: [PATCH 1/7] poc for optimizing the life cycle dialog --- .../basic-range-date-picker.component.ts | 9 ++++++++- .../src/app/shared/components/datepicker/datepicker.ts | 2 ++ .../sections/edit_dialog_component.html.erb | 5 +++-- 3 files changed, 13 insertions(+), 3 deletions(-) diff --git a/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts b/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts index d9c791513467..56e261aa7fb4 100644 --- a/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts +++ b/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts @@ -184,6 +184,7 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce showMonths: 2, onReady: (_date:Date[], _datestr:string, instance:flatpickr.Instance) => { instance.calendarContainer.classList.add('op-datepicker-modal--flatpickr-instance'); + instance.calendarContainer.setAttribute('popover', ''); }, onChange: (dates:Date[], dateStr:string) => { if (dates.length === 2) { @@ -195,6 +196,12 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce this.cdRef.detectChanges(); }, + onOpen: (_date:Date[], _datestr:string, instance:flatpickr.Instance) => { + setTimeout(() => { + instance.calendarContainer.showPopover(); + instance.calendarContainer.style.marginTop = '0'; + }); + }, onDayCreate: async (dObj:Date[], dStr:string, fp:flatpickr.Instance, dayElem:DayElement) => { onDayCreate( dayElem, @@ -203,7 +210,7 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce !!this.minimalDate && dayElem.dateObj <= this.minimalDate, ); }, - static: this.inDialog, + static: false, //this.inDialog, }, this.input.nativeElement as HTMLInputElement, ); diff --git a/frontend/src/app/shared/components/datepicker/datepicker.ts b/frontend/src/app/shared/components/datepicker/datepicker.ts index ed56654f52f7..426d0a363224 100644 --- a/frontend/src/app/shared/components/datepicker/datepicker.ts +++ b/frontend/src/app/shared/components/datepicker/datepicker.ts @@ -136,6 +136,8 @@ export class DatePicker { }, dateFormat: this.datepickerFormat, defaultDate: this.date, + position: 'auto', + appendTo: document.querySelector('#edit-project-life-cycles-dialog'), locale: { weekdays: { shorthand: this.I18n.t('date.abbr_day_names'), diff --git a/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb b/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb index a2eab213ea09..b62b1d9786fb 100644 --- a/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb +++ b/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb @@ -1,7 +1,8 @@ <%= render(Primer::Alpha::Dialog.new(title: t("label_life_cycle_step_plural"), - size: :large, - id: dialog_id)) do |d| + size: :auto, + id: dialog_id, + style: "max-width: 650px")) do |d| d.with_header(variant: :large) d.with_body(classes: "Overlay-body_autocomplete_height") do render(::ProjectLifeCycles::Sections::EditComponent.new(model)) From ad8b7f5f34ed43a1b06ff3cf40136f39ff3326b4 Mon Sep 17 00:00:00 2001 From: ulferts Date: Thu, 19 Dec 2024 11:59:16 +0100 Subject: [PATCH 2/7] use standard dialog size --- .../sections/edit_dialog_component.html.erb | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb b/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb index b62b1d9786fb..97353efd9045 100644 --- a/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb +++ b/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb @@ -1,10 +1,9 @@ <%= render(Primer::Alpha::Dialog.new(title: t("label_life_cycle_step_plural"), - size: :auto, - id: dialog_id, - style: "max-width: 650px")) do |d| + size: :medium_portrait, + id: dialog_id)) do |d| d.with_header(variant: :large) - d.with_body(classes: "Overlay-body_autocomplete_height") do + d.with_body do render(::ProjectLifeCycles::Sections::EditComponent.new(model)) end d.with_footer do From 40d54c4236d4194e5002fe5b5b4d69b38e1cc0ea Mon Sep 17 00:00:00 2001 From: ulferts Date: Thu, 19 Dec 2024 20:50:56 +0100 Subject: [PATCH 3/7] concentrate top-layer handling in single method --- .../basic-range-date-picker.component.html | 1 + .../basic-range-date-picker.component.ts | 31 ++++++++++++++----- 2 files changed, 24 insertions(+), 8 deletions(-) diff --git a/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.html b/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.html index e083c264ab13..05fe101b3ce4 100644 --- a/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.html +++ b/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.html @@ -16,6 +16,7 @@ [ngModel]="stringValue" (ngModelChange)="changeValueFromInputDebounced($event)" (focus)="showDatePicker()" + (click)="sentCalendarToTopLayer()" /> { instance.calendarContainer.classList.add('op-datepicker-modal--flatpickr-instance'); - instance.calendarContainer.setAttribute('popover', ''); }, onChange: (dates:Date[], dateStr:string) => { if (dates.length === 2) { @@ -196,12 +198,6 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce this.cdRef.detectChanges(); }, - onOpen: (_date:Date[], _datestr:string, instance:flatpickr.Instance) => { - setTimeout(() => { - instance.calendarContainer.showPopover(); - instance.calendarContainer.style.marginTop = '0'; - }); - }, onDayCreate: async (dObj:Date[], dStr:string, fp:flatpickr.Instance, dayElem:DayElement) => { onDayCreate( dayElem, @@ -241,4 +237,23 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce private resolveDateArrayToString(dates:string[]):string { return dates.join(` ${rangeSeparator} `); } + + // In case the date picker is opened in a dialog, it needs to be in the top layer + // since the dialog is also there. This method is called in two cases: + // 1. When the date picker is opened + // 2. When the date picker is already opened and the user clicks on the input + // The later is necessary as otherwise the date picker would be removed from the top layer + // when the user clicks on the input. That is because the input is not part of the date picker + // so clicking on it would be considered a click on the backdrop which removes an item from + // the top layer again. + public sentCalendarToTopLayer() { + if (!this.datePickerInstance.isOpen || !this.inDialog) { + return; + } + + const calendarContainer = this.datePickerInstance.datepickerInstance.calendarContainer; + calendarContainer.setAttribute('popover', ''); + calendarContainer.showPopover(); + calendarContainer.style.marginTop = '0'; + } } From cd0a3e5a2eb4668cf504769c594d5001c72313be Mon Sep 17 00:00:00 2001 From: ulferts Date: Thu, 19 Dec 2024 20:52:30 +0100 Subject: [PATCH 4/7] avoid unnecessary opening when clicking The browser is already opened on focus --- .../basic-range-date-picker/basic-range-date-picker.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts b/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts index 897f37c56068..cc40739f993b 100644 --- a/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts +++ b/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts @@ -185,6 +185,7 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce allowInput: true, mode: 'range', showMonths: 2, + clickOpens: false, onReady: (_date:Date[], _datestr:string, instance:flatpickr.Instance) => { instance.calendarContainer.classList.add('op-datepicker-modal--flatpickr-instance'); }, From 03fa133a5fa4a755892c367163ae860a10cb839d Mon Sep 17 00:00:00 2001 From: ulferts Date: Thu, 19 Dec 2024 21:19:02 +0100 Subject: [PATCH 5/7] generalize solution to single date picker --- app/forms/projects/life_cycles/form.rb | 2 +- .../basic-range-date-picker.component.ts | 13 ++++++- .../basic-single-date-picker.component.html | 1 + .../basic-single-date-picker.component.ts | 37 +++++++++++++++++-- .../components/datepicker/datepicker.ts | 2 - .../sections/edit_dialog_component.html.erb | 4 +- .../sections/edit_dialog_component.rb | 6 +-- 7 files changed, 52 insertions(+), 13 deletions(-) diff --git a/app/forms/projects/life_cycles/form.rb b/app/forms/projects/life_cycles/form.rb index 1bd83b7b1f61..0783a0a85090 100644 --- a/app/forms/projects/life_cycles/form.rb +++ b/app/forms/projects/life_cycles/form.rb @@ -53,7 +53,7 @@ def base_input_attributes label: "#{icon} #{text}".html_safe, # rubocop:disable Rails/OutputSafety leading_visual: { icon: :calendar }, datepicker_options: { - inDialog: true, + inDialog: ProjectLifeCycles::Sections::EditDialogComponent::DIALOG_ID, data: { action: "change->overview--project-life-cycles-form#handleChange" } }, wrapper_data_attributes: { diff --git a/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts b/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts index cc40739f993b..95aec0c4a068 100644 --- a/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts +++ b/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts @@ -115,7 +115,7 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce @Input() inputClassNames = ''; - @Input() inDialog = false; + @Input() inDialog:string; @Input() dataAction = ''; @@ -207,7 +207,8 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce !!this.minimalDate && dayElem.dateObj <= this.minimalDate, ); }, - static: false, //this.inDialog, + static: false, + appendTo: this.appendToBodyOrDialog(), }, this.input.nativeElement as HTMLInputElement, ); @@ -257,4 +258,12 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce calendarContainer.showPopover(); calendarContainer.style.marginTop = '0'; } + + private appendToBodyOrDialog():HTMLElement|undefined { + if (this.inDialog) { + return document.querySelector(`#${this.inDialog}`) as HTMLElement; + } + + return undefined; + } } diff --git a/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.html b/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.html index 436cf1bccd7b..3e7fe6910308 100644 --- a/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.html +++ b/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.html @@ -16,6 +16,7 @@ [disabled]="disabled" (input)="changeValueFromInput($event.target.value)" (focus)="showDatePicker()" + (click)="sentCalendarToTopLayer()" [attr.data-remote-field-key]="remoteFieldKey" /> diff --git a/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts b/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts index 1ecc03b2fe52..658cea065bf1 100644 --- a/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts +++ b/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts @@ -95,7 +95,7 @@ export class OpBasicSingleDatePickerComponent implements ControlValueAccessor, O @Input() remoteFieldKey = null; - @Input() inDialog = false; + @Input() inDialog:string; @Input() dataAction = ''; @@ -146,7 +146,10 @@ export class OpBasicSingleDatePickerComponent implements ControlValueAccessor, O } showDatePicker():void { - this.datePickerInstance?.show(); + if (!this.datePickerInstance?.isOpen) { + this.datePickerInstance?.show(); + this.sentCalendarToTopLayer(); + } } private initializeDatePicker() { @@ -182,7 +185,8 @@ export class OpBasicSingleDatePickerComponent implements ControlValueAccessor, O !!this.minimalDate && dayElem.dateObj <= this.minimalDate, ); }, - static: this.inDialog, + static: false, + appendTo: this.appendToBodyOrDialog(), }, this.input.nativeElement as HTMLInputElement, ); @@ -204,4 +208,31 @@ export class OpBasicSingleDatePickerComponent implements ControlValueAccessor, O registerOnTouched(fn:(_:string) => void):void { this.onTouched = fn; } + + // In case the date picker is opened in a dialog, it needs to be in the top layer + // since the dialog is also there. This method is called in two cases: + // 1. When the date picker is opened + // 2. When the date picker is already opened and the user clicks on the input + // The later is necessary as otherwise the date picker would be removed from the top layer + // when the user clicks on the input. That is because the input is not part of the date picker + // so clicking on it would be considered a click on the backdrop which removes an item from + // the top layer again. + public sentCalendarToTopLayer() { + if (!this.datePickerInstance.isOpen || !this.inDialog) { + return; + } + + const calendarContainer = this.datePickerInstance.datepickerInstance.calendarContainer; + calendarContainer.setAttribute('popover', ''); + calendarContainer.showPopover(); + calendarContainer.style.marginTop = '0'; + } + + private appendToBodyOrDialog():HTMLElement|undefined { + if (this.inDialog) { + return document.querySelector(`#${this.inDialog}`) as HTMLElement; + } + + return undefined; + } } diff --git a/frontend/src/app/shared/components/datepicker/datepicker.ts b/frontend/src/app/shared/components/datepicker/datepicker.ts index 426d0a363224..ed56654f52f7 100644 --- a/frontend/src/app/shared/components/datepicker/datepicker.ts +++ b/frontend/src/app/shared/components/datepicker/datepicker.ts @@ -136,8 +136,6 @@ export class DatePicker { }, dateFormat: this.datepickerFormat, defaultDate: this.date, - position: 'auto', - appendTo: document.querySelector('#edit-project-life-cycles-dialog'), locale: { weekdays: { shorthand: this.I18n.t('date.abbr_day_names'), diff --git a/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb b/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb index 97353efd9045..b7d6fd2db05e 100644 --- a/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb +++ b/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.html.erb @@ -1,7 +1,7 @@ <%= render(Primer::Alpha::Dialog.new(title: t("label_life_cycle_step_plural"), size: :medium_portrait, - id: dialog_id)) do |d| + id: DIALOG_ID)) do |d| d.with_header(variant: :large) d.with_body do render(::ProjectLifeCycles::Sections::EditComponent.new(model)) @@ -10,7 +10,7 @@ component_collection do |footer_collection| footer_collection.with_component(Primer::ButtonComponent.new( data: { - "close-dialog-id": dialog_id + "close-dialog-id": DIALOG_ID } )) do t("button_cancel") diff --git a/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.rb b/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.rb index 45f2689421ea..9d9a248eda15 100644 --- a/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.rb +++ b/modules/overviews/app/components/project_life_cycles/sections/edit_dialog_component.rb @@ -1,3 +1,5 @@ +# frozen_string_literal: true + #-- copyright # OpenProject is an open source project management software. # Copyright (C) the OpenProject GmbH @@ -33,9 +35,7 @@ class EditDialogComponent < ApplicationComponent include OpTurbo::Streamable include OpPrimer::ComponentHelpers - def dialog_id - "edit-project-life-cycles-dialog" - end + DIALOG_ID = "edit-project-life-cycles-dialog" end end end From 89a780c909902bb1f05e458d98d14f95e5270b97 Mon Sep 17 00:00:00 2001 From: ulferts Date: Thu, 19 Dec 2024 21:19:51 +0100 Subject: [PATCH 6/7] avoid unnecessary opening when clicking The date picker is already opened on focus --- .../basic-single-date-picker.component.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts b/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts index 658cea065bf1..c167b11f829f 100644 --- a/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts +++ b/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts @@ -161,6 +161,7 @@ export class OpBasicSingleDatePickerComponent implements ControlValueAccessor, O allowInput: true, mode: 'single', showMonths: 1, + clickOpens: false, onReady: (_date:Date[], _datestr:string, instance:flatpickr.Instance) => { instance.calendarContainer.classList.add('op-datepicker-modal--flatpickr-instance'); }, From c5388334b5c94fe3c359c5ef79acd009e2550ada Mon Sep 17 00:00:00 2001 From: ulferts Date: Fri, 20 Dec 2024 23:04:06 +0100 Subject: [PATCH 7/7] add another top layer transportation call --- .../basic-range-date-picker.component.ts | 4 +++- .../basic-single-date-picker.component.ts | 4 +++- .../components/projects/project_life_cycles/edit_dialog.rb | 6 ++++-- 3 files changed, 10 insertions(+), 4 deletions(-) diff --git a/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts b/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts index 95aec0c4a068..5f2e7e6a0983 100644 --- a/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts +++ b/frontend/src/app/shared/components/datepicker/basic-range-date-picker/basic-range-date-picker.component.ts @@ -185,7 +185,6 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce allowInput: true, mode: 'range', showMonths: 2, - clickOpens: false, onReady: (_date:Date[], _datestr:string, instance:flatpickr.Instance) => { instance.calendarContainer.classList.add('op-datepicker-modal--flatpickr-instance'); }, @@ -199,6 +198,9 @@ export class OpBasicRangeDatePickerComponent implements OnInit, ControlValueAcce this.cdRef.detectChanges(); }, + onOpen: () => { + this.sentCalendarToTopLayer(); + }, onDayCreate: async (dObj:Date[], dStr:string, fp:flatpickr.Instance, dayElem:DayElement) => { onDayCreate( dayElem, diff --git a/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts b/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts index c167b11f829f..b73dd1a714bb 100644 --- a/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts +++ b/frontend/src/app/shared/components/datepicker/basic-single-date-picker/basic-single-date-picker.component.ts @@ -161,7 +161,6 @@ export class OpBasicSingleDatePickerComponent implements ControlValueAccessor, O allowInput: true, mode: 'single', showMonths: 1, - clickOpens: false, onReady: (_date:Date[], _datestr:string, instance:flatpickr.Instance) => { instance.calendarContainer.classList.add('op-datepicker-modal--flatpickr-instance'); }, @@ -178,6 +177,9 @@ export class OpBasicSingleDatePickerComponent implements ControlValueAccessor, O this.cdRef.detectChanges(); }, + onOpen: () => { + this.sentCalendarToTopLayer(); + }, onDayCreate: async (_dObj:Date[], _dStr:string, _fp:flatpickr.Instance, dayElem:DayElement) => { onDayCreate( dayElem, diff --git a/spec/support/components/projects/project_life_cycles/edit_dialog.rb b/spec/support/components/projects/project_life_cycles/edit_dialog.rb index 83c1a3fce9f8..fc6e218d02f5 100644 --- a/spec/support/components/projects/project_life_cycles/edit_dialog.rb +++ b/spec/support/components/projects/project_life_cycles/edit_dialog.rb @@ -50,10 +50,12 @@ def within_async_content(close_after_yield: false, &) end def set_date_for(step, value:) + dialog_selector = "##{::ProjectLifeCycles::Sections::EditDialogComponent::DIALOG_ID}" + datepicker = if value.is_a?(Array) - Components::RangeDatepicker.new + Components::RangeDatepicker.new(dialog_selector) else - Components::BasicDatepicker.new + Components::BasicDatepicker.new(dialog_selector) end datepicker.open(