From 3cf3cfa33e4dffc03e099b34cd2458eea2e5a210 Mon Sep 17 00:00:00 2001 From: Marien Fressinaud Date: Tue, 8 Nov 2022 16:59:14 +0100 Subject: [PATCH] tec: Render the full ticket page when submitting messages In the next commit, we'll be able to change the status of the ticket when answering. Since the status appears at the top of the page, it needs to be rendered in the response. This brings some difficulties because I no longer use `` and now the scroll is set to the top of the page at the end of the submit. Also, there are two cases to handle: 1. the data are valid and we redirect to the "show" page: in this case, a new `data-turbo-preserve-scroll` attribute allows to cancel the scroll of the page 2. an error in the form (e.g. empty content) is detected and we render a bad request response: in this case, the `data-turbo-preserve-scroll` solution doesn't work (i.e. the Turbo `currentVisit` doesn't exist), so we generate an `autofocus` attribute in order to scroll to the form. The user experience is somewhat degraded, but I think it's good enough, at least for a MVP. --- assets/javascripts/application.js | 24 +++++ .../controllers/tinymce_controller.js | 6 ++ src/Controller/Tickets/MessagesController.php | 12 +-- .../tickets/messages/_messages.html.twig | 96 ------------------- templates/tickets/show.html.twig | 90 ++++++++++++++++- .../Tickets/MessagesControllerTest.php | 4 +- 6 files changed, 126 insertions(+), 106 deletions(-) delete mode 100644 templates/tickets/messages/_messages.html.twig diff --git a/assets/javascripts/application.js b/assets/javascripts/application.js index b46778dc..fb28c6c2 100644 --- a/assets/javascripts/application.js +++ b/assets/javascripts/application.js @@ -13,3 +13,27 @@ const application = Application.start(); application.register('color-scheme', ColorSchemeController); application.register('popup', PopupController); application.register('tinymce', TinymceController); + +// Allow to disable the automatic scroll-to-top on form submission. +// Submitting forms with a `data-turbo-preserve-scroll` attribute will keep the +// scroll position at the current position. +let disableScroll = false; + +document.addEventListener('turbo:submit-start', (event) => { + if (event.detail.formSubmission.formElement.hasAttribute('data-turbo-preserve-scroll')) { + disableScroll = true; + } +}); + +document.addEventListener('turbo:before-render', (event) => { + if (disableScroll && Turbo.navigator.currentVisit) { + // As explained on GitHub, `Turbo.navigator.currentVisit.scrolled` + // is internal and private attribute: we should NOT access it. + // Unfortunately, there is no good alternative yet to maintain the + // scroll position. This means we have to be pay double attention when + // upgrading Turbo. + // Reference: https://github.com/hotwired/turbo/issues/37#issuecomment-979466543 + Turbo.navigator.currentVisit.scrolled = true; + disableScroll = false; + } +}); diff --git a/assets/javascripts/controllers/tinymce_controller.js b/assets/javascripts/controllers/tinymce_controller.js index 9825f96a..72b43a72 100644 --- a/assets/javascripts/controllers/tinymce_controller.js +++ b/assets/javascripts/controllers/tinymce_controller.js @@ -13,6 +13,11 @@ export default class extends Controller { language = null; // Use the default language (en_US) } + let autofocus = ''; + if (this.element.autofocus) { + autofocus = this.element.id; + } + const configuration = { selector: '#' + this.element.id, skin: colorScheme === 'light' ? 'oxide' : 'oxide-dark', @@ -28,6 +33,7 @@ export default class extends Controller { promotion: false, link_assume_external_targets: true, link_target_list: false, + auto_focus: autofocus, }; window.tinymce.init(configuration); diff --git a/src/Controller/Tickets/MessagesController.php b/src/Controller/Tickets/MessagesController.php index 4996ab74..1e0842ba 100644 --- a/src/Controller/Tickets/MessagesController.php +++ b/src/Controller/Tickets/MessagesController.php @@ -38,9 +38,10 @@ public function create( $csrfToken = $request->request->get('_csrf_token', ''); if (!$this->isCsrfTokenValid('create ticket message', $csrfToken)) { - return $this->renderBadRequest('tickets/messages/_messages.html.twig', [ + return $this->renderBadRequest('tickets/show.html.twig', [ 'ticket' => $ticket, 'messages' => $ticket->getMessages(), + 'organization' => $ticket->getOrganization(), 'message' => $messageContent, 'error' => $this->csrfError(), ]); @@ -57,9 +58,10 @@ public function create( $errors = $validator->validate($message); if (count($errors) > 0) { - return $this->renderBadRequest('tickets/messages/_messages.html.twig', [ + return $this->renderBadRequest('tickets/show.html.twig', [ 'ticket' => $ticket, 'messages' => $ticket->getMessages(), + 'organization' => $ticket->getOrganization(), 'message' => $messageContent, 'errors' => $this->formatErrors($errors), ]); @@ -67,10 +69,8 @@ public function create( $messageRepository->save($message, true); - return $this->render('tickets/messages/_messages.html.twig', [ - 'ticket' => $ticket, - 'messages' => $ticket->getMessages(), - 'message' => '', + return $this->redirectToRoute('ticket', [ + 'uid' => $ticket->getUid(), ]); } } diff --git a/templates/tickets/messages/_messages.html.twig b/templates/tickets/messages/_messages.html.twig deleted file mode 100644 index f629d5b0..00000000 --- a/templates/tickets/messages/_messages.html.twig +++ /dev/null @@ -1,96 +0,0 @@ -{# - # This file is part of Bileto. - # Copyright 2022 Probesys - # SPDX-License-Identifier: AGPL-3.0-or-later - #} - - -
-
- {% for message in messages %} -
-
- {{ icon('circle-user') }} - {% if message.createdBy == ticket.requester %} - - {{ icon('user') }} - - {{ 'Requester' | trans }} - - - {% elseif message.createdBy == ticket.assignee %} - - {{ icon('headset') }} - - {{ 'Assignee' | trans }} - - - {% endif %} -
- -
-
-
- {{ message.createdBy.email }} -
- -
- -
- {{ message.createdAt.format('Y-m-d') }} -
-
- -
- {{ message.content | raw }} -
-
-
- {% endfor %} -
- -
- - - {% if error %} - - {% endif %} - -
- {% if errors.content is defined %} - - {% endif %} - - -
- -
- -
-
-
- diff --git a/templates/tickets/show.html.twig b/templates/tickets/show.html.twig index 49b57756..98caad05 100644 --- a/templates/tickets/show.html.twig +++ b/templates/tickets/show.html.twig @@ -38,8 +38,94 @@
-
- {{ include('tickets/messages/_messages.html.twig') }} +
+
+ {% for message in messages %} +
+
+ {{ icon('circle-user') }} + {% if message.createdBy == ticket.requester %} + + {{ icon('user') }} + + {{ 'Requester' | trans }} + + + {% elseif message.createdBy == ticket.assignee %} + + {{ icon('headset') }} + + {{ 'Assignee' | trans }} + + + {% endif %} +
+ +
+
+
+ {{ message.createdBy.email }} +
+ +
+ +
+ {{ message.createdAt.format('Y-m-d') }} +
+
+ +
+ {{ message.content | raw }} +
+
+
+ {% endfor %} +
+ +
+ + + {% if error %} + + {% endif %} + +
+ {% if errors.content is defined %} + + {% endif %} + + +
+ +
+ +
+
diff --git a/tests/Controller/Tickets/MessagesControllerTest.php b/tests/Controller/Tickets/MessagesControllerTest.php index 3f2c9420..0f552ef1 100644 --- a/tests/Controller/Tickets/MessagesControllerTest.php +++ b/tests/Controller/Tickets/MessagesControllerTest.php @@ -19,7 +19,7 @@ class MessagesControllerTest extends WebTestCase use Factories; use ResetDatabase; - public function testPostCreateCreatesAMessageAndRenderMessages(): void + public function testPostCreateCreatesAMessageAndRedirects(): void { $now = new \DateTimeImmutable('2022-11-02'); Time::freeze($now); @@ -41,7 +41,7 @@ public function testPostCreateCreatesAMessageAndRenderMessages(): void Time::unfreeze(); $this->assertSame(1, MessageFactory::count()); - $this->assertResponseIsSuccessful(); + $this->assertResponseRedirects("/tickets/{$ticket->getUid()}", 302); $message = MessageFactory::first(); $this->assertSame($messageContent, $message->getContent()); $this->assertEquals($now, $message->getCreatedAt());