From e62ab09d5954ef1c75f35f2a809d68acbbc09c58 Mon Sep 17 00:00:00 2001 From: AdrienClairembault Date: Wed, 18 Dec 2024 14:35:30 +0100 Subject: [PATCH] Apply suggested UI changes --- js/modules/Forms/EditorController.js | 21 +++++++- .../conditional_visibility_dropdown.html.twig | 23 ++++++--- .../pages/admin/form/form_comment.html.twig | 50 +++++++++++++------ .../pages/admin/form/form_question.html.twig | 48 ++++++++++++++---- .../pages/admin/form/form_section.html.twig | 16 +++++- .../cypress/e2e/form/editor/conditions.cy.js | 17 ++++--- tests/cypress/e2e/form/editor/editor.cy.js | 14 +++--- tests/cypress/e2e/form/form_preview.cy.js | 2 +- 8 files changed, 141 insertions(+), 50 deletions(-) diff --git a/js/modules/Forms/EditorController.js b/js/modules/Forms/EditorController.js index 65facb264e6..f4f9592507e 100644 --- a/js/modules/Forms/EditorController.js +++ b/js/modules/Forms/EditorController.js @@ -180,7 +180,7 @@ export class GlpiFormEditorController $(document) .on( 'show.bs.dropdown', - '[data-glpi-form-editor-visibility-editor-dropdown]', + '[data-glpi-form-editor-visibility-dropdown]', (e) => this.#renderVisibilityEditor( $(e.target) .parent() @@ -431,6 +431,12 @@ export class GlpiFormEditorController ); break; + case "show-visibility-dropdown": + this.#showVisibilityDropdown( + target.closest('[data-glpi-form-editor-block],[data-glpi-form-editor-section-details]') + ); + break; + // Set the conditional visibility of a section/question/comment case "set-visiblity-value": { const input = $(`#${target.attr('for')}`); @@ -1877,6 +1883,19 @@ export class GlpiFormEditorController this.#setActiveItem(form_details); } + #showVisibilityDropdown(container) { + container + .find('[data-glpi-form-editor-visibility-dropdown-container]') + .removeClass('d-none') + ; + + const dropdown = container + .find('[data-glpi-form-editor-visibility-dropdown-container]') + .find('[data-glpi-form-editor-visibility-dropdown]') + ; + bootstrap.Dropdown.getOrCreateInstance(dropdown[0]).show(); + } + #setVisibilityValue(container, value) { // Show/hide badges in the container container.find('[data-glpi-editor-visibility-badge]') diff --git a/templates/pages/admin/form/conditional_visibility_dropdown.html.twig b/templates/pages/admin/form/conditional_visibility_dropdown.html.twig index 6c5de330858..b0c7b1136ea 100644 --- a/templates/pages/admin/form/conditional_visibility_dropdown.html.twig +++ b/templates/pages/admin/form/conditional_visibility_dropdown.html.twig @@ -37,15 +37,22 @@ {% set question_strategy = item.getConfiguredVisibilityStrategy() %} {% endif %} +{# Spacing div (ms-auto must be applied to an item that is always visible) #} +
+
@@ -53,10 +60,10 @@ title="{{ __('Configure visibility') }}" data-bs-toggle="dropdown" data-bs-auto-close="outside" - class="dropdown-toggle btn btn-outline-secondary btn-pill btn-sm px-2" + class="dropdown-toggle btn btn-outline-secondary btn-sm px-2" data-bs-placement="top" type="button" - data-glpi-form-editor-visibility-editor-dropdown + data-glpi-form-editor-visibility-dropdown > {% for strategy in enum_cases('Glpi\\Form\\QuestionVisibilityStrategy') %} {% set is_visible = question_strategy == strategy %} @@ -80,7 +87,7 @@ {{ __('Conditional visibility') }} -
+
{# Display strategy picker #} {% for strategy in enum_cases('Glpi\\Form\\QuestionVisibilityStrategy') %} {% set rand = random() %} @@ -110,7 +117,7 @@ {% set is_visible = question_strategy.showEditor() %} {% set visibility_class = is_visible ? '' : 'd-none' %}
+ + {# Delete comment #} + + + {# Extra actions #} +
+ +
    +
  • + +
    • +
+
{# Display common fields #} @@ -124,20 +160,6 @@ }) ) }}
- -
- -
{# Common hidden data #} diff --git a/templates/pages/admin/form/form_question.html.twig b/templates/pages/admin/form/form_question.html.twig index e6ceacde86a..0f17b314bc1 100644 --- a/templates/pages/admin/form/form_question.html.twig +++ b/templates/pages/admin/form/form_question.html.twig @@ -108,6 +108,7 @@ {# Visibility dropdown #} {{ include('pages/admin/form/conditional_visibility_dropdown.html.twig', { 'item': question, + 'type': "Glpi\\Form\\Question", }, with_context = false) }} {# Duplicate question #} @@ -119,8 +120,42 @@ title="{{ __("Duplicate question") }}" data-glpi-form-editor-on-click="duplicate-question" data-glpi-form-editor-question-extra-details + > + + {# Delete question #} + + + {# Extra actions #} +
+ +
    +
  • + +
    • +
+
{# Render the specific question type #} @@ -215,11 +250,11 @@ ) }} {# Render the specific question options #} -
+
{{ question_type.renderAdministrationOptionsTemplate(question)|raw }}
-
diff --git a/templates/pages/admin/form/form_section.html.twig b/templates/pages/admin/form/form_section.html.twig index 55ebfd3b339..87641b68298 100644 --- a/templates/pages/admin/form/form_section.html.twig +++ b/templates/pages/admin/form/form_section.html.twig @@ -95,6 +95,7 @@ {# Visibility dropdown #} {{ include('pages/admin/form/conditional_visibility_dropdown.html.twig', { 'item': section, + 'type': "Glpi\\Form\\Section", }, with_context = false) }} {# Collapse section #} @@ -108,13 +109,13 @@ > {# Extra actions #} -
+
  • @@ -161,6 +162,17 @@ {{ __("Delete section") }}
    • + +
  • + +
diff --git a/tests/cypress/e2e/form/editor/conditions.cy.js b/tests/cypress/e2e/form/editor/conditions.cy.js index 22470cccc53..4ebd0617f47 100644 --- a/tests/cypress/e2e/form/editor/conditions.cy.js +++ b/tests/cypress/e2e/form/editor/conditions.cy.js @@ -104,6 +104,11 @@ function checkThatVisibilityOptionsAreHidden() { cy.findByRole('label', {'name': "Hidden if..."}).should('not.exist'); } +function initVisibilityConfiguration() { + cy.findByRole('button', {'name': 'More actions'}).click(); + cy.findByRole('button', {'name': 'Configure visiblity'}).click(); +} + function openVisibilityOptions() { cy.findByTitle('Configure visibility').click(); } @@ -193,7 +198,7 @@ describe ('Conditions', () => { // Select 'Visible if...' (editor should be displayed) getAndFocusQuestion('My first question').within(() => { checkThatVisibilityOptionsAreHidden(); - openVisibilityOptions(); + initVisibilityConfiguration(); checkThatVisibilityOptionsAreVisible(); checkThatSelectedVisibilityOptionIs('Always visible'); checkThatConditionEditorIsNotDisplayed(); @@ -241,7 +246,7 @@ describe ('Conditions', () => { }); saveAndReload(); getAndFocusQuestion('My first question').within(() => { - openVisibilityOptions(); + initVisibilityConfiguration(); checkThatSelectedVisibilityOptionIs('Always visible'); checkThatConditionEditorIsNotDisplayed(); closeVisibilityOptions(); @@ -256,7 +261,7 @@ describe ('Conditions', () => { saveAndReload(); getAndFocusQuestion('My third question').within(() => { - openVisibilityOptions(); + initVisibilityConfiguration(); setVisibilityOption('Visible if...'); fillCondition(0, null, 'My second question', 'Is not equal to', 'I love GLPI'); addNewEmptyCondition(); @@ -312,7 +317,7 @@ describe ('Conditions', () => { addQuestion('My third question'); getAndFocusQuestion('My third question').within(() => { - openVisibilityOptions(); + initVisibilityConfiguration(); setVisibilityOption('Visible if...'); fillCondition(0, null, 'My second question', 'Is not equal to', 'I love GLPI'); addNewEmptyCondition(); @@ -367,7 +372,7 @@ describe ('Conditions', () => { saveAndReload(); getAndFocusComment('My first comment').within(() => { - openVisibilityOptions(); + initVisibilityConfiguration(); setVisibilityOption('Visible if...'); fillCondition(0, null, 'My second question', 'Contains', 'I love GLPI'); addNewEmptyCondition(); @@ -422,7 +427,7 @@ describe ('Conditions', () => { saveAndReload(); getAndFocusSection('My second section').within(() => { - openVisibilityOptions(); + initVisibilityConfiguration(); setVisibilityOption('Visible if...'); fillCondition(0, null, 'My second question', 'Do not contains', 'I love GLPI'); addNewEmptyCondition(); diff --git a/tests/cypress/e2e/form/editor/editor.cy.js b/tests/cypress/e2e/form/editor/editor.cy.js index d852be94b03..4b981b5a3bd 100644 --- a/tests/cypress/e2e/form/editor/editor.cy.js +++ b/tests/cypress/e2e/form/editor/editor.cy.js @@ -273,7 +273,7 @@ describe ('Form editor', () => { // Delete question cy.get('@second_section') - .findByRole('button', {'name': "Section actions"}) + .findByRole('button', {'name': "More actions"}) .click() ; cy.findByRole('button', {'name': "Delete section"}).click(); @@ -301,7 +301,7 @@ describe ('Form editor', () => { // Duplicate second section cy.get('@sections').eq(1).as('second_section'); cy.get('@second_section') - .findByRole('button', {'name': "Section actions"}) + .findByRole('button', {'name': "More actions"}) .click() ; cy.findByRole('button', {'name': "Duplicate section"}).click(); @@ -359,7 +359,7 @@ describe ('Form editor', () => { // Merge the two sections cy.get('@sections') .eq(1) - .findByRole('button', {'name': "Section actions"}) + .findByRole('button', {'name': "More actions"}) .click() ; cy.findByRole('button', {'name': "Merge with previous section"}).click(); @@ -530,7 +530,7 @@ describe ('Form editor', () => { // Create a second section cy.addSection("Second section"); - cy.findAllByRole('region', {'name': 'Form section'}).as('sections'); + cy.findAllByRole('region', {'name': 'Section details'}).as('sections'); cy.get('@sections').should('have.length', 2); // Add two questions to our section @@ -540,7 +540,7 @@ describe ('Form editor', () => { // Open "reorder sections" modal cy.get('@sections') .eq(0) - .findByRole('button', {'name': "Section actions"}) + .findByRole('button', {'name': "More actions"}) .click() ; cy.findByRole('button', {'name': "Move section"}).click(); @@ -719,8 +719,8 @@ describe ('Form editor', () => { cy.getDropdownByLabelText("Question type").selectDropdownValue('Actors'); // Duplicate first section - cy.findAllByRole('region', {'name': 'Form section'}).eq(0).within(() => { - cy.findByRole('button', {'name': 'Section actions'}).click(); + cy.findAllByRole('region', {'name': 'Section details'}).eq(0).within(() => { + cy.findByRole('button', {'name': 'More actions'}).click(); cy.findByRole('button', {'name': 'Duplicate section'}).click(); }); diff --git a/tests/cypress/e2e/form/form_preview.cy.js b/tests/cypress/e2e/form/form_preview.cy.js index d9098aa4609..50eeb9c3d70 100644 --- a/tests/cypress/e2e/form/form_preview.cy.js +++ b/tests/cypress/e2e/form/form_preview.cy.js @@ -126,7 +126,7 @@ describe('Form preview', config, () => { cy.findAllByRole('region', { 'name': 'Form section' }).eq(1).within(() => { // Set the section description - cy.findByRole('button', { 'name': 'Section actions' }).click(); + cy.findByRole('button', { 'name': 'More actions' }).click(); cy.findByRole('button', { 'name': 'Merge with previous section' }).click(); }); checkPreviewButton();