Skip to content

Commit

Permalink
Apply suggestions from code review
Browse files Browse the repository at this point in the history
  • Loading branch information
@ccailly
ccailly committed Dec 23, 2024
1 parent 82ef5bf commit fada622
Show file tree
Hide file tree
Showing 5 changed files with 110 additions and 39 deletions.
73 changes: 53 additions & 20 deletions css/includes/components/form/_form-editor-horizontal-layout.scss
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,6 @@
}
}
}

.glpi-form-editor-drag-question-placeholder {
height: unset !important;
margin-bottom: 0.5rem !important;
}
}


Expand Down Expand Up @@ -112,16 +107,26 @@
}
}
}

[data-glpi-form-editor-horizontal-block-placeholder] {
&:not([data-glpi-form-editor-active-horizontal-block-placeholder]) {
[data-glpi-form-editor-toolbar] {
display: none !important;
}
}
}
}
}

&:not([data-glpi-form-editor-active-horizontal-blocks]) > [data-glpi-form-editor-toolbar] {
position: absolute;
height: 0;
width: 0;
opacity: 0;
visibility: hidden;
overflow: hidden;
&:not([data-glpi-form-editor-active-horizontal-blocks]) {
> [data-glpi-form-editor-toolbar] {
position: absolute;
height: 0;
width: 0;
opacity: 0;
visibility: hidden;
overflow: hidden;
}
}

[data-glpi-form-editor-horizontal-blocks] {
Expand All @@ -134,7 +139,8 @@
@for $i from 2 through 4 {
&:has(> section:nth-of-type(#{$i})) {
[data-glpi-form-editor-block],
[data-glpi-form-editor-horizontal-block-placeholder] {
[data-glpi-form-editor-horizontal-block-placeholder],
.glpi-form-editor-drag-question-placeholder {
&:not([data-glpi-form-editor-active-question]):not([data-glpi-form-editor-active-comment]) {
// Ensure each block has the same max width and share the available space
@if $i == 4 {
Expand Down Expand Up @@ -184,21 +190,32 @@
margin-top: auto;
margin-bottom: auto;
order: 100;

// Remove hover effect on buttons
button {
border: unset !important;
background-color: unset !important;
color: var(--tblr-btn-color) !important;
}
}

[data-glpi-form-editor-horizontal-block-placeholder] {
// Hide the placeholder toolbar when the block isn't active
// Hide the placeholder toolbar when the block isn't active and the placeholder isn't hovered
&:not([data-glpi-form-editor-active-horizontal-block-placeholder]) {
[data-glpi-form-editor-toolbar] {
display: none !important;
&:not(:hover) {
[data-glpi-form-editor-toolbar] {
display: none !important;
}
}
}

border: 2px dashed var(--tblr-border-color);
border-radius: var(--tblr-border-radius);
&:not(:has(.glpi-form-editor-drag-question-placeholder)) {
border: 2px dashed var(--tblr-border-color);
border-radius: var(--tblr-border-radius);
}

min-height: 100px;
flex: 1;
order: 10;
display: flex;
justify-content: center;
align-items: center;
Expand All @@ -210,11 +227,27 @@
@include media-breakpoint-down(sm) {
flex-direction: column;
}

// Remove hover effect on buttons
button {
border: unset !important;
background-color: unset !important;
color: var(--tblr-btn-color) !important;
}
}

.glpi-form-editor-drag-question-placeholder {
width: 100%;
max-width: unset !important;
height: 100% !important;
margin: unset !important;
}
}

.glpi-form-editor-drag-question-placeholder {
width: 10%;
flex: 1;
height: unset !important;
margin: unset !important;
}
}
}
39 changes: 21 additions & 18 deletions js/modules/Forms/EditorController.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -483,10 +483,14 @@ export class GlpiFormEditorController

// If the item is a horizontal block, we need to find all questions and comments
if (is_horizontal_block) {
blocks = $(item).find("[data-glpi-form-editor-block]");
blocks = $(item).find("[data-glpi-form-editor-block], [data-glpi-form-editor-horizontal-block-placeholder]");
}

blocks.each((horizontal_rank, block) => {
if ($(block).is("[data-glpi-form-editor-horizontal-block-placeholder]")) {
return;
}

// Determine the type of the block
const itemType = $(block).is("[data-glpi-form-editor-question]") ? 'question' : 'comment';

Expand Down Expand Up @@ -841,11 +845,9 @@ export class GlpiFormEditorController
destination = target;
action = "append";
} else if (target.data('glpi-form-editor-horizontal-block-placeholder') !== undefined) {
// Adding a block at the end of a horizontal block
destination = target
.closest("[data-glpi-form-editor-horizontal-blocks-container]")
.find("[data-glpi-form-editor-horizontal-blocks]");
action = "append";
// Adding a block just after the horizontal layout placeholder
destination = target;
action = "after";

// Remove the placeholder just after adding the block
setTimeout(() => this.#removeHorizontalLayoutSlot(target), 0);
Expand Down Expand Up @@ -1590,14 +1592,14 @@ export class GlpiFormEditorController
sections
.each((index, section) => {
const blocks_container = $(section)
.find("[data-glpi-form-editor-section-blocks], [data-glpi-form-editor-horizontal-blocks], [data-glpi-form-editor-question-drag-merge]");
.find("[data-glpi-form-editor-section-blocks], [data-glpi-form-editor-horizontal-blocks], [data-glpi-form-editor-question-drag-merge], [data-glpi-form-editor-horizontal-block-placeholder]");

sortable(blocks_container, {
// Drag and drop handle selector
handle: '[data-glpi-form-editor-question-handle]',

// Restrict sortable items
items: '[data-glpi-form-editor-block]',
items: '[data-glpi-form-editor-block], [data-glpi-form-editor-horizontal-block-placeholder]',

// Limit the number of blocks in horizontal blocks
maxItems: blocks_container.attr("data-glpi-form-editor-horizontal-blocks") !== typeof undefined ? 4 : 0,
Expand All @@ -1606,13 +1608,13 @@ export class GlpiFormEditorController
acceptFrom: '[data-glpi-form-editor-section-blocks], [data-glpi-form-editor-horizontal-blocks]',

// Placeholder class
placeholderClass: 'glpi-form-editor-drag-question-placeholder',
placeholder: '<section class="glpi-form-editor-drag-question-placeholder"></section>',
});
});

// Keep track on unsaved changes if the sort order was updated
sections
.find("[data-glpi-form-editor-section-blocks], [data-glpi-form-editor-horizontal-blocks], [data-glpi-form-editor-question-drag-merge]")
.find("[data-glpi-form-editor-section-blocks], [data-glpi-form-editor-horizontal-blocks], [data-glpi-form-editor-question-drag-merge], [data-glpi-form-editor-horizontal-block-placeholder]")
.on('sortupdate', (e) => {
// Trigger an action to make sure we use the main entry point
// where common action related functions are excuted
Expand All @@ -1629,7 +1631,7 @@ export class GlpiFormEditorController
// Run the post move process if any item was dragged, even if it was not
// moved in the end (= dragged on itself)
sections
.find("[data-glpi-form-editor-section-blocks], [data-glpi-form-editor-horizontal-blocks], [data-glpi-form-editor-question-drag-merge]")
.find("[data-glpi-form-editor-section-blocks], [data-glpi-form-editor-horizontal-blocks], [data-glpi-form-editor-question-drag-merge], [data-glpi-form-editor-horizontal-block-placeholder]")
.on('sortstop', (e) => {
// The 'sortstop' event trigger twice for a single drag and drop
// action.
Expand All @@ -1648,11 +1650,12 @@ export class GlpiFormEditorController
);
}

if (
$(e.detail.item).parent().data('glpi-form-editor-horizontal-blocks') !== undefined
&& $(e.detail.item).parent().find("[data-glpi-form-editor-horizontal-block-placeholder]").length > 0
) {
$(e.detail.item).parent().find("[data-glpi-form-editor-horizontal-block-placeholder]").first().remove();
// Handle case where the item was dragged in a placeholder
// This is a special case where the item is not moved but replace the placeholder
if ($(e.detail.item).parent().data('glpi-form-editor-horizontal-block-placeholder') !== undefined) {
const placeholder = $(e.detail.item).parent();
$(e.detail.item).insertAfter(placeholder);
placeholder.remove();
}

// Handle case where the item was dragged in a drag and merge area
Expand Down Expand Up @@ -1991,8 +1994,8 @@ export class GlpiFormEditorController

const new_placeholder = this.#addBlock(target, template);

// Enable sortable on the new placeholder
this.#enableSortable(new_placeholder);
// Enable sortable
this.#enableSortable(target);

// Set new placeholder as active
this.#setActiveItem(new_placeholder);
Expand Down
11 changes: 11 additions & 0 deletions templates/pages/admin/form/form_horizontal_block.html.twig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,20 @@
data-glpi-form-editor-horizontal-blocks
aria-label="{{ __("Horizontal blocks") }}"
>
{% set previous_index = -1 %}
{% for form_block in blocks %}
{# Only 4 blocks are allowed in a horizontal layout #}
{% if loop.index <= 4 %}
{% if form_block.fields.horizontal_rank > previous_index + 1 %}
{% for i in 1..(form_block.fields.horizontal_rank - (previous_index + 1)) %}
{{ include('pages/admin/form/form_horizontal_block_placeholder.html.twig', {
'can_update' : can_update,
'form' : form,
}, with_context = false) }}
{% endfor %}
{% endif %}
{% set previous_index = form_block.fields.horizontal_rank %}

{{ form_block.displayBlockForEditor() }}
{% endif %}
{% endfor %}
Expand Down
17 changes: 16 additions & 1 deletion templates/pages/admin/form/form_horizontal_block_toolbar.html.twig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@

{% if can_update %}
<div
class="d-flex justify-content-center h-full"
class="btn-group-vertical h-full"
style="box-shadow:none;"
role="group"
data-glpi-form-editor-horizontal-block-toolbar
Expand All @@ -55,5 +55,20 @@
<i class="ti ti-square-plus"></i>
</span>
</button>

{# Remove horizontal layout action #}
<button
type="button"
class="btn btn-icon btn-ghost-danger flex-grow-0"
data-bs-toggle="tooltip"
data-bs-placement="bottom"
title="{{ __("Remove horizontal layout") }}"
data-glpi-form-editor-on-click="delete-horizontal-layout"
type="button"
>
<span class="px-2 d-flex align-items-center">
<i class="ti ti-trash"></i>
</span>
</button>
</div>
{% endif %}
9 changes: 9 additions & 0 deletions templates/pages/form_renderer.html.twig
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@

{#
# ---------------------------------------------------------------------
#
Expand Down Expand Up @@ -98,7 +99,15 @@
{% endif %}

<section data-glpi-form-renderer-horizontal-blocks>
{% set previous_index = -1 %}
{% for form_block in form_group_blocks %}
{% if form_block.fields.horizontal_rank > previous_index + 1 %}
{% for i in 1..(form_block.fields.horizontal_rank - (previous_index + 1)) %}
<div style="flex: 1;"></div>
{% endfor %}
{% endif %}
{% set previous_index = form_block.fields.horizontal_rank %}

{% if form_block is instanceof('Glpi\\Form\\Question') %}
{% set question_type = form_block.getQuestionType() %}
{% endif %}
Expand Down

0 comments on commit fada622

Please sign in to comment.