Skip to content

Commit 2e317d5

Browse files
froozeifyfroozeify
authored
fix(form): Forms with many destinations may overload the browser
1 parent f897c85 commit 2e317d5

File tree

7 files changed

+227
-34
lines changed

7 files changed

+227
-34
lines changed

‎js/modules/Forms/DestinationAccordionController.js‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
/**
2+
* ---------------------------------------------------------------------
3+
*
4+
* GLPI - Gestionnaire Libre de Parc Informatique
5+
*
6+
* http://glpi-project.org
7+
*
8+
* @copyright 2015-2025 Teclib' and contributors.
9+
* @licence https://www.gnu.org/licenses/gpl-3.0.html
10+
*
11+
* ---------------------------------------------------------------------
12+
*
13+
* LICENSE
14+
*
15+
* This file is part of GLPI.
16+
*
17+
* This program is free software: you can redistribute it and/or modify
18+
* it under the terms of the GNU General Public License as published by
19+
* the Free Software Foundation, either version 3 of the License, or
20+
* (at your option) any later version.
21+
*
22+
* This program is distributed in the hope that it will be useful,
23+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
24+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
25+
* GNU General Public License for more details.
26+
*
27+
* You should have received a copy of the GNU General Public License
28+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
29+
*
30+
* ---------------------------------------------------------------------
31+
*/
32+
import { GlpiFormDestinationAutoConfigController } from "./DestinationAutoConfigController.js";
33+
import { GlpiFormDestinationConditionController } from "./DestinationConditionController.js";
34+
35+
export class GlpiFormDestinationAccordionController
36+
{
37+
constructor() {
38+
this.#watchForAccordionToggle();
39+
}
40+
41+
triggerWatchers() {
42+
new GlpiFormDestinationAutoConfigController();
43+
new GlpiFormDestinationConditionController();
44+
}
45+
46+
#watchForAccordionToggle() {
47+
const accordionWrapper = document.querySelector('#glpi-destinations-accordion');
48+
49+
accordionWrapper.addEventListener('show.bs.collapse', async (e) => {
50+
const accordionItem = e.target;
51+
const accordionItemContent = accordionItem.querySelector('.accordion-body');
52+
if (accordionItemContent.innerHTML.trim() !== '') {
53+
return;
54+
}
55+
56+
accordionItemContent.innerHTML = '<div class="text-center"><div class="spinner-border text-primary mb-3" role="status"></div></div>';
57+
58+
const content = await $.ajax({
59+
url: `${CFG_GLPI.root_doc}/Form/${accordionItem.dataset.form}/Destinations/${accordionItem.dataset.formDestination}`,
60+
method: 'GET',
61+
});
62+
63+
// Note: must use `$().html` to make sure we trigger scripts
64+
$(accordionItemContent).html(content);
65+
66+
// We trigger the watcher
67+
this.triggerWatchers();
68+
});
69+
}
70+
}

‎src/Glpi/Controller/Form/Destination/GetDestinationFormController.php‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
<?php
2+
3+
/**
4+
* ---------------------------------------------------------------------
5+
*
6+
* GLPI - Gestionnaire Libre de Parc Informatique
7+
*
8+
* http://glpi-project.org
9+
*
10+
* @copyright 2015-2025 Teclib' and contributors.
11+
* @licence https://www.gnu.org/licenses/gpl-3.0.html
12+
*
13+
* ---------------------------------------------------------------------
14+
*
15+
* LICENSE
16+
*
17+
* This file is part of GLPI.
18+
*
19+
* This program is free software: you can redistribute it and/or modify
20+
* it under the terms of the GNU General Public License as published by
21+
* the Free Software Foundation, either version 3 of the License, or
22+
* (at your option) any later version.
23+
*
24+
* This program is distributed in the hope that it will be useful,
25+
* but WITHOUT ANY WARRANTY; without even the implied warranty of
26+
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
27+
* GNU General Public License for more details.
28+
*
29+
* You should have received a copy of the GNU General Public License
30+
* along with this program. If not, see <https://www.gnu.org/licenses/>.
31+
*
32+
* ---------------------------------------------------------------------
33+
*/
34+
35+
namespace Glpi\Controller\Form\Destination;
36+
37+
use Glpi\Application\View\TemplateRenderer;
38+
use Glpi\Controller\AbstractController;
39+
use Glpi\Exception\Http\AccessDeniedHttpException;
40+
use Glpi\Exception\Http\BadRequestHttpException;
41+
use Glpi\Form\Destination\FormDestination;
42+
use Symfony\Component\HttpFoundation\Request;
43+
use Symfony\Component\HttpFoundation\Response;
44+
use Symfony\Component\Routing\Attribute\Route;
45+
46+
final class GetDestinationFormController extends AbstractController
47+
{
48+
#[Route("/Form/{form_id}/Destinations/{destination_id}", name: "glpi_form_destination_get_form", methods: "GET")]
49+
public function __invoke(Request $request, int $form_id, int $destination_id): Response
50+
{
51+
$destination = new FormDestination();
52+
$loaded = $destination->getFromDB($destination_id);
53+
if (!$loaded) {
54+
throw new BadRequestHttpException();
55+
}
56+
57+
// Right check
58+
if (!$destination->can($destination_id, READ)) {
59+
throw new AccessDeniedHttpException();
60+
}
61+
62+
$twig = TemplateRenderer::getInstance()->render('pages/admin/form/form_destination_form.html.twig', [
63+
'destination' => $destination,
64+
'form' => $destination->getForm(),
65+
'can_update' => FormDestination::canUpdate() && $destination->canUpdateItem(),
66+
'concrete_destination' => $destination->getConcreteDestinationItem(),
67+
]);
68+
return new Response($twig);
69+
}
70+
}

‎src/Glpi/Form/Destination/FormDestination.php‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,12 +110,17 @@ public static function displayTabContentForItem(
110110
return false;
111111
}
112112

113+
$destinations = $item->getDestinations();
114+
115+
$active = null;
113116
// Reopen the active accordion item
114117
if (isset($_SESSION['active_destination'])) {
115118
$active = $_SESSION['active_destination'];
116119
unset($_SESSION['active_destination']);
117120
} else {
118-
$active = null;
121+
if (count($destinations) === 1) {
122+
$active = $destinations[array_key_first($destinations)]->getID();
123+
}
119124
}
120125

121126
$manager = FormDestinationManager::getInstance();
@@ -125,7 +130,7 @@ public static function displayTabContentForItem(
125130
'icon' => self::getIcon(),
126131
'form' => $item,
127132
'default_destination_object' => $manager->getDefaultType(),
128-
'destinations' => $item->getDestinations(),
133+
'destinations' => $destinations,
129134
'available_destinations_types' => $manager->getDestinationTypesDropdownValues(),
130135
'active_destination' => $active,
131136
'can_update' => self::canUpdate(),

‎templates/pages/admin/form/form_destination.html.twig‎

Lines changed: 18 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,6 @@
3636
{# @var \Glpi\Form\Desination\FormDestination destinations #}
3737
{# @var int|null active_destination #}
3838
{# @var boolean can_update #}
39-
4039
<div class="py-2 px-3">
4140
<h2 class="d-flex">
4241
<i class="{{ icon }} me-2"></i>
@@ -86,11 +85,11 @@
8685
{% else %}
8786
<section class="accordion mt-4" id="glpi-destinations-accordion" aria-label="{{ __("Form destinations") }}">
8887
{% for destination in destinations %}
89-
{% set is_expanded = active_destination == destination.getID() or destinations|length == 1 %}
88+
{% set is_expanded = active_destination == destination.getID() %}
9089
{% set concrete_destination = destination.getConcreteDestinationItem() %}
9190
{% set rand = random() %}
9291
<section
93-
class="accordion-item"
92+
class="accordion-item destination-{{ destination.getID() }}"
9493
aria-label="{{ destination.getName() }}"
9594
>
9695
<div class="accordion-header">
@@ -141,29 +140,18 @@
141140
id="glpi-destinations-accordion-collapse-{{ rand }}-{{ loop.index }}"
142141
class="accordion-collapse collapse {{ is_expanded ? 'show' : '' }}"
143142
data-bs-parent="#glpi-destinations-accordion"
143+
data-form="{{ form.getID() }}"
144+
data-form-destination="{{ destination.getID() }}"
144145
>
145146
<div class="accordion-body p-0">
146-
<form id="form-destination-{{ destination.getID() }}">
147-
<div class="overflow-x-hidden px-4">
148-
{{ concrete_destination.renderConfigForm(
149-
form,
150-
destination,
151-
destination.getConfig(),
152-
)|raw }}
153-
{% if concrete_destination.useDefaultConfigLayout() and can_update %}
154-
<div class="mt-3 mb-3">
155-
{{ include('pages/admin/form/form_destination_actions.html.twig', {
156-
form: form,
157-
destination: destination
158-
}, with_context = false) }}
159-
</div>
160-
{% endif %}
161-
</div>
162-
163-
{# Hidden values #}
164-
<input type="hidden" name="id" value="{{ destination.getID() }}"/>
165-
<input type="hidden" name="_glpi_csrf_token" value="{{ csrf_token() }}"/>
166-
</form>
147+
{% if is_expanded %}
148+
{{ include('pages/admin/form/form_destination_form.html.twig', {
149+
destination,
150+
form,
151+
can_update,
152+
concrete_destination,
153+
}) }}
154+
{% endif %}
167155
</div>
168156
</div>
169157
</section>
@@ -173,11 +161,11 @@
173161
</div>
174162

175163
<script>
176-
import("/js/modules/Forms/DestinationAutoConfigController.js").then((m) => {
177-
new m.GlpiFormDestinationAutoConfigController();
178-
});
164+
import("/js/modules/Forms/DestinationAccordionController.js").then(function (module) {
165+
const accordionController = new module.GlpiFormDestinationAccordionController();
179166
180-
import("/js/modules/Forms/DestinationConditionController.js").then((m) => {
181-
new m.GlpiFormDestinationConditionController();
182-
});
167+
{% if active_destination %}
168+
accordionController.triggerWatchers();
169+
{% endif %}
170+
})
183171
</script>

‎templates/pages/admin/form/form_destination_form.html.twig‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
{#
2+
# ---------------------------------------------------------------------
3+
#
4+
# GLPI - Gestionnaire Libre de Parc Informatique
5+
#
6+
# http://glpi-project.org
7+
#
8+
# @copyright 2015-2025 Teclib' and contributors.
9+
# @licence https://www.gnu.org/licenses/gpl-3.0.html
10+
#
11+
# ---------------------------------------------------------------------
12+
#
13+
# LICENSE
14+
#
15+
# This file is part of GLPI.
16+
#
17+
# This program is free software: you can redistribute it and/or modify
18+
# it under the terms of the GNU General Public License as published by
19+
# the Free Software Foundation, either version 3 of the License, or
20+
# (at your option) any later version.
21+
#
22+
# This program is distributed in the hope that it will be useful,
23+
# but WITHOUT ANY WARRANTY; without even the implied warranty of
24+
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
25+
# GNU General Public License for more details.
26+
#
27+
# You should have received a copy of the GNU General Public License
28+
# along with this program. If not, see <https://www.gnu.org/licenses/>.
29+
#
30+
# ---------------------------------------------------------------------
31+
#}
32+
33+
{#
34+
REQUIRED:
35+
- `destination`: FormDestination
36+
- `form`: Form
37+
- `can_update`: bool
38+
- `concrete_destination`: FormDestinationInterface|null
39+
#}
40+
<form id="form-destination-{{ destination.getID() }}">
41+
<div class="overflow-x-hidden px-4">
42+
{{ concrete_destination.renderConfigForm(
43+
form,
44+
destination,
45+
destination.getConfig(),
46+
)|raw }}
47+
{% if concrete_destination.useDefaultConfigLayout() and can_update %}
48+
<div class="mt-3 mb-3">
49+
{{ include('pages/admin/form/form_destination_actions.html.twig', {
50+
form: form,
51+
destination: destination
52+
}, with_context = false) }}
53+
</div>
54+
{% endif %}
55+
</div>
56+
57+
{# Hidden values #}
58+
<input type="hidden" name="id" value="{{ destination.getID() }}"/>
59+
<input type="hidden" name="_glpi_csrf_token" value="{{ csrf_token() }}"/>
60+
</form>

‎tests/cypress/e2e/form/destination_config_fields/request_type.cy.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ describe('Request type configuration', () => {
3939
cy.createFormWithAPI().visitFormTab('Form');
4040
cy.findByRole('button', {'name': "Add a question"}).click();
4141
cy.focused().type("My request type question");
42-
cy.getDropdownByLabelText('Question type').selectDropdownValue('Request type');
42+
cy.findByRole('option', {'name': 'New question'}).changeQuestionType('Request type');
4343
cy.findByRole('button', {'name': 'Save'}).click();
4444
cy.checkAndCloseAlert('Item successfully updated');
4545

‎tests/cypress/e2e/form/form_destination.cy.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -239,7 +239,7 @@ describe('Form destination', () => {
239239
.click();
240240

241241
cy.findByRole("textbox", {name: "Form destination name"}).should('have.value', 'Original destination');
242-
cy.findByRole('region', {name: 'Title configuration'}).awaitTinyMCE().as("original_title_field");
242+
cy.findByRole('region', {name: 'Title configuration'}).as("original_title_field");
243243
cy.get("@original_title_field").should('contain.text', 'Custom title for duplication test');
244244
});
245245
});

0 commit comments

Comments
 (0)