Skip to content

Commit

Permalink
tec: Render the full ticket page when submitting messages
Browse files Browse the repository at this point in the history 
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 `<turbo-frame>`
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.
  • Loading branch information
@marien-probesys
marien-probesys committed Nov 8, 2022
1 parent 732baa1 commit 3cf3cfa
Show file tree
Hide file tree
Showing 6 changed files with 126 additions and 106 deletions.
24 changes: 24 additions & 0 deletions assets/javascripts/application.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
});
6 changes: 6 additions & 0 deletions assets/javascripts/controllers/tinymce_controller.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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);
Expand Down
12 changes: 6 additions & 6 deletions src/Controller/Tickets/MessagesController.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
]);
Expand All @@ -57,20 +58,19 @@ 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),
]);
}

$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(),
]);
}
}
96 changes: 0 additions & 96 deletions templates/tickets/messages/_messages.html.twig
View file

This file was deleted.

90 changes: 88 additions & 2 deletions templates/tickets/show.html.twig
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,94 @@
</div>

<div class="ticket__layout grid">
<div class="ticket__timeline">
{{ include('tickets/messages/_messages.html.twig') }}
<div class="ticket__timeline flow-large">
<div class="flow-large timeline">
{% for message in messages %}
<article class="message">
<div class="message__avatar">
{{ icon('circle-user') }}
{% if message.createdBy == ticket.requester %}
<span class="message__role">
{{ icon('user') }}
<span class="sr-only">
{{ 'Requester' | trans }}
</span>
</span>
{% elseif message.createdBy == ticket.assignee %}
<span class="message__role">
{{ icon('headset') }}
<span class="sr-only">
{{ 'Assignee' | trans }}
</span>
</span>
{% endif %}
</div>

<div class="message__box">
<div class="message__top">
<div class="message__author">
{{ message.createdBy.email }}
</div>

<div class="message__top-separator"></div>

<div class="message__date">
{{ message.createdAt.format('Y-m-d') }}
</div>
</div>

<div class="message__content flow">
{{ message.content | raw }}
</div>
</div>
</article>
{% endfor %}
</div>

<form
action="{{ path('create ticket message', {'uid': ticket.uid}) }}"
method="post"
class="wrapper wrapper--center flow"
data-turbo-preserve-scroll
>
<input type="hidden" name="_csrf_token" value="{{ csrf_token('create ticket message') }}">

{% if error %}
<div class="alert alert--error" role="alert" data-turbo-cache="false" data-test="alert-error">
<div class="alert__title">{{ 'Error' | trans }}</div>

<p class="alert__message">
{{ error }}
</p>
</div>
{% endif %}

<div class="flow-small">
{% if errors.content is defined %}
<p class="form__error" role="alert" id="message-error">
<span class="sr-only">{{ 'Error' | trans }}</span>
{{ errors.content }}
</p>
{% endif %}

<textarea
id="message"
name="message"
data-controller="tinymce"
{% if errors.content is defined %}
autofocus
aria-invalid="true"
aria-errormessage="message-error"
{% endif %}
>{{ message }}</textarea>
</div>

<div class="form__actions">
<button id="form-create-message-submit" class="button--primary" type="submit">
{{ 'Answer' | trans }}
</button>
</div>
</form>
</div>

<div class="ticket__info flow-larger">
Expand Down
4 changes: 2 additions & 2 deletions tests/Controller/Tickets/MessagesControllerTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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());
Expand Down

0 comments on commit 3cf3cfa

Please sign in to comment.