From 14d9c386fd05cbd0dd37253806137a09c3dec324 Mon Sep 17 00:00:00 2001
From: Gusted <postmaster@gusted.xyz>
Date: Tue, 16 Jul 2024 21:30:47 +0200
Subject: [PATCH] [UI] Fix HTMX support for profile card
- There were two issues with the profile card since the introduction of
HTMX in 3e8414179c3f3e8a12d3a66fdf32c144f941f5c3. If an HTMX request
resulted in a flash message, it wasn't being shown and HTMX was
replacing all the HTML content instead of morphing it into the existing
DOM which caused event listeners to be lost for buttons.
- Flash messages are now properly being shown by using `hx-swap-oob`
and sending the alerts on a HTMX request, this does mean it requires
server-side changes in order to support HTMX requests like this, but
it's luckily not a big change either.
- Morphing is now enabled for the profile card by setting
`hx-swap="morph"`, and weirdly, the morphing library was already
installed and included as a dependency. This solves the issue of buttons
losing their event listeners.
- This patch also adds HTMX support to the modals feature, which means
that the blocking feature on the profile card now takes advantage of
HTMX.
- Added a E2E test.
---
routers/web/user/profile.go | 15 ++-----
templates/base/alert.tmpl | 11 +++--
templates/shared/user/profile_big_avatar.tmpl | 14 ++++---
tests/e2e/profile_actions.test.e2e.js | 41 +++++++++++++++++++
tests/integration/block_test.go | 23 ++++-------
web_src/js/features/common-global.js | 23 +++++++++--
6 files changed, 86 insertions(+), 41 deletions(-)
create mode 100644 tests/e2e/profile_actions.test.e2e.js
diff --git a/routers/web/user/profile.go b/routers/web/user/profile.go
index a83d7f7333..24b0db2f1d 100644
--- a/routers/web/user/profile.go
+++ b/routers/web/user/profile.go
@@ -341,7 +341,6 @@ func prepareUserProfileTabData(ctx *context.Context, showPrivate bool, profileDb
// Action response for follow/unfollow user request
func Action(ctx *context.Context) {
var err error
- var redirectViaJSON bool
action := ctx.FormString("action")
if ctx.ContextUser.IsOrganization() && (action == "block" || action == "unblock") {
@@ -357,10 +356,8 @@ func Action(ctx *context.Context) {
err = user_model.UnfollowUser(ctx, ctx.Doer.ID, ctx.ContextUser.ID)
case "block":
err = user_service.BlockUser(ctx, ctx.Doer.ID, ctx.ContextUser.ID)
- redirectViaJSON = true
case "unblock":
err = user_model.UnblockUser(ctx, ctx.Doer.ID, ctx.ContextUser.ID)
- redirectViaJSON = true
}
if err != nil {
@@ -371,21 +368,15 @@ func Action(ctx *context.Context) {
}
if ctx.ContextUser.IsOrganization() {
- ctx.Flash.Error(ctx.Tr("org.follow_blocked_user"))
+ ctx.Flash.Error(ctx.Tr("org.follow_blocked_user"), true)
} else {
- ctx.Flash.Error(ctx.Tr("user.follow_blocked_user"))
+ ctx.Flash.Error(ctx.Tr("user.follow_blocked_user"), true)
}
}
- if redirectViaJSON {
- ctx.JSON(http.StatusOK, map[string]any{
- "redirect": ctx.ContextUser.HomeLink(),
- })
- return
- }
-
if ctx.ContextUser.IsIndividual() {
shared_user.PrepareContextForProfileBigAvatar(ctx)
+ ctx.Data["IsHTMX"] = true
ctx.HTML(http.StatusOK, tplProfileBigAvatar)
return
} else if ctx.ContextUser.IsOrganization() {
diff --git a/templates/base/alert.tmpl b/templates/base/alert.tmpl
index 760d3bfa2c..b2deab5c2d 100644
--- a/templates/base/alert.tmpl
+++ b/templates/base/alert.tmpl
@@ -1,20 +1,23 @@
{{if .Flash.ErrorMsg}}
- <div class="ui negative message flash-message flash-error">
+ <div id="flash-message" class="ui negative message flash-message flash-error" hx-swap-oob="true">
<p>{{.Flash.ErrorMsg | SanitizeHTML}}</p>
</div>
{{end}}
{{if .Flash.SuccessMsg}}
- <div class="ui positive message flash-message flash-success">
+ <div id="flash-message" class="ui positive message flash-message flash-success" hx-swap-oob="true">
<p>{{.Flash.SuccessMsg | SanitizeHTML}}</p>
</div>
{{end}}
{{if .Flash.InfoMsg}}
- <div class="ui info message flash-message flash-info">
+ <div id="flash-message" class="ui info message flash-message flash-info" hx-swap-oob="true">
<p>{{.Flash.InfoMsg | SanitizeHTML}}</p>
</div>
{{end}}
{{if .Flash.WarningMsg}}
- <div class="ui warning message flash-message flash-warning">
+ <div id="flash-message" class="ui warning message flash-message flash-warning" hx-swap-oob="true">
<p>{{.Flash.WarningMsg | SanitizeHTML}}</p>
</div>
{{end}}
+{{if and (not .Flash.ErrorMsg) (not .Flash.SuccessMsg) (not .Flash.InfoMsg) (not .Flash.WarningMsg) (not .IsHTMX)}}
+ <div id="flash-message" hx-swap-oob="true"></div>
+{{end}}
diff --git a/templates/shared/user/profile_big_avatar.tmpl b/templates/shared/user/profile_big_avatar.tmpl
index 3063aeacf7..6795eaed2c 100644
--- a/templates/shared/user/profile_big_avatar.tmpl
+++ b/templates/shared/user/profile_big_avatar.tmpl
@@ -1,4 +1,7 @@
-<div id="profile-avatar-card" class="ui card">
+{{if .IsHTMX}}
+ {{template "base/alert" .}}
+{{end}}
+<div id="profile-avatar-card" class="ui card" hx-swap="morph">
<div id="profile-avatar" class="content tw-flex">
{{if eq .SignedUserID .ContextUser.ID}}
<a class="image" href="{{AppSubUrl}}/user/settings" data-tooltip-content="{{ctx.Locale.Tr "user.change_avatar"}}">
@@ -98,7 +101,7 @@
</li>
{{end}}
{{if and .IsSigned (ne .SignedUserID .ContextUser.ID)}}
- <li class="follow" hx-target="#profile-avatar-card" hx-indicator="#profile-avatar-card" >
+ <li class="follow" hx-target="#profile-avatar-card" hx-indicator="#profile-avatar-card">
{{if $.IsFollowing}}
<button hx-post="{{.ContextUser.HomeLink}}?action=unfollow" class="ui basic red button">
{{svg "octicon-person"}} {{ctx.Locale.Tr "user.unfollow"}}
@@ -109,14 +112,13 @@
</button>
{{end}}
</li>
- <li class="block">
+ <li class="block" hx-target="#profile-avatar-card" hx-indicator="#profile-avatar-card">
{{if $.IsBlocked}}
- <button class="ui basic red button link-action" data-url="{{.ContextUser.HomeLink}}?action=unblock&redirect_to={{$.Link}}">
+ <button class="ui basic red button" hx-post="{{.ContextUser.HomeLink}}?action=unblock">
{{svg "octicon-person"}} {{ctx.Locale.Tr "user.unblock"}}
</button>
{{else}}
- <button type="submit" class="ui basic orange button delete-button"
- data-modal-id="block-user" data-url="{{.ContextUser.HomeLink}}?action=block">
+ <button type="submit" class="ui basic orange button" data-modal-id="block-user" hx-post="{{.ContextUser.HomeLink}}?action=block" hx-confirm="-">
{{svg "octicon-blocked"}} {{ctx.Locale.Tr "user.block"}}
</button>
{{end}}
diff --git a/tests/e2e/profile_actions.test.e2e.js b/tests/e2e/profile_actions.test.e2e.js
new file mode 100644
index 0000000000..20155b8df4
--- /dev/null
+++ b/tests/e2e/profile_actions.test.e2e.js
@@ -0,0 +1,41 @@
+// @ts-check
+import {test, expect} from '@playwright/test';
+import {login_user, load_logged_in_context} from './utils_e2e.js';
+
+test('Follow actions', async ({browser}, workerInfo) => {
+ await login_user(browser, workerInfo, 'user2');
+ const context = await load_logged_in_context(browser, workerInfo, 'user2');
+ const page = await context.newPage();
+
+ await page.goto('/user1');
+ await page.waitForLoadState('networkidle');
+
+ // Check if following and then unfollowing works.
+ // This checks that the event listeners of
+ // the buttons aren't dissapearing.
+ const followButton = page.locator('.follow');
+ await expect(followButton).toContainText('Follow');
+ await followButton.click();
+ await expect(followButton).toContainText('Unfollow');
+ await followButton.click();
+ await expect(followButton).toContainText('Follow');
+
+ // Simple block interaction.
+ await expect(page.locator('.block')).toContainText('Block');
+
+ await page.locator('.block').click();
+ await expect(page.locator('#block-user')).toBeVisible();
+ await page.locator('#block-user .ok').click();
+ await expect(page.locator('.block')).toContainText('Unblock');
+ await expect(page.locator('#block-user')).not.toBeVisible();
+
+ // Check that following the user yields in a error being shown.
+ await followButton.click();
+ const flashMessage = page.locator('#flash-message');
+ await expect(flashMessage).toBeVisible();
+ await expect(flashMessage).toContainText('You cannot follow this user because you have blocked this user or this user has blocked you.');
+
+ // Unblock interaction.
+ await page.locator('.block').click();
+ await expect(page.locator('.block')).toContainText('Block');
+});
diff --git a/tests/integration/block_test.go b/tests/integration/block_test.go
index 8f40ed13e8..f917d700da 100644
--- a/tests/integration/block_test.go
+++ b/tests/integration/block_test.go
@@ -34,15 +34,8 @@ func BlockUser(t *testing.T, doer, blockedUser *user_model.User) {
"_csrf": GetCSRF(t, session, "/"+blockedUser.Name),
"action": "block",
})
- resp := session.MakeRequest(t, req, http.StatusOK)
+ session.MakeRequest(t, req, http.StatusOK)
- type redirect struct {
- Redirect string `json:"redirect"`
- }
-
- var respBody redirect
- DecodeJSON(t, resp, &respBody)
- assert.EqualValues(t, "/"+blockedUser.Name, respBody.Redirect)
assert.True(t, unittest.BeanExists(t, &user_model.BlockedUser{BlockID: blockedUser.ID, UserID: doer.ID}))
}
@@ -303,11 +296,10 @@ func TestBlockActions(t *testing.T) {
"_csrf": GetCSRF(t, session, "/"+blockedUser.Name),
"action": "follow",
})
- session.MakeRequest(t, req, http.StatusOK)
+ resp := session.MakeRequest(t, req, http.StatusOK)
- flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
- assert.NotNil(t, flashCookie)
- assert.EqualValues(t, "error%3DYou%2Bcannot%2Bfollow%2Bthis%2Buser%2Bbecause%2Byou%2Bhave%2Bblocked%2Bthis%2Buser%2Bor%2Bthis%2Buser%2Bhas%2Bblocked%2Byou.", flashCookie.Value)
+ htmlDoc := NewHTMLParser(t, resp.Body)
+ assert.Contains(t, htmlDoc.Find("#flash-message").Text(), "You cannot follow this user because you have blocked this user or this user has blocked you.")
// Assert it still doesn't exist.
unittest.AssertNotExistsBean(t, &user_model.Follow{UserID: doer.ID, FollowID: blockedUser.ID})
@@ -323,11 +315,10 @@ func TestBlockActions(t *testing.T) {
"_csrf": GetCSRF(t, session, "/"+doer.Name),
"action": "follow",
})
- session.MakeRequest(t, req, http.StatusOK)
+ resp := session.MakeRequest(t, req, http.StatusOK)
- flashCookie := session.GetCookie(forgejo_context.CookieNameFlash)
- assert.NotNil(t, flashCookie)
- assert.EqualValues(t, "error%3DYou%2Bcannot%2Bfollow%2Bthis%2Buser%2Bbecause%2Byou%2Bhave%2Bblocked%2Bthis%2Buser%2Bor%2Bthis%2Buser%2Bhas%2Bblocked%2Byou.", flashCookie.Value)
+ htmlDoc := NewHTMLParser(t, resp.Body)
+ assert.Contains(t, htmlDoc.Find("#flash-message").Text(), "You cannot follow this user because you have blocked this user or this user has blocked you.")
unittest.AssertNotExistsBean(t, &user_model.Follow{UserID: blockedUser.ID, FollowID: doer.ID})
})
diff --git a/web_src/js/features/common-global.js b/web_src/js/features/common-global.js
index e7db9b2336..5a304d96f5 100644
--- a/web_src/js/features/common-global.js
+++ b/web_src/js/features/common-global.js
@@ -295,11 +295,11 @@ async function linkAction(e) {
export function initGlobalLinkActions() {
function showDeletePopup(e) {
e.preventDefault();
- const $this = $(this);
+ const $this = $(this || e.target);
const dataArray = $this.data();
let filter = '';
- if (this.getAttribute('data-modal-id')) {
- filter += `#${this.getAttribute('data-modal-id')}`;
+ if ($this[0].getAttribute('data-modal-id')) {
+ filter += `#${$this[0].getAttribute('data-modal-id')}`;
}
const $dialog = $(`.delete.modal${filter}`);
@@ -317,6 +317,10 @@ export function initGlobalLinkActions() {
$($this.data('form')).trigger('submit');
return;
}
+ if ($this[0].getAttribute('hx-confirm')) {
+ e.detail.issueRequest(true);
+ return;
+ }
const postData = new FormData();
for (const [key, value] of Object.entries(dataArray)) {
if (key && key.startsWith('data')) {
@@ -338,6 +342,19 @@ export function initGlobalLinkActions() {
// Helpers.
$('.delete-button').on('click', showDeletePopup);
+
+ document.addEventListener('htmx:confirm', (e) => {
+ e.preventDefault();
+ // htmx:confirm is triggered for every HTMX request, even those that don't
+ // have the `hx-confirm` attribute specified. To avoid opening modals for
+ // those elements, check if 'e.detail.question' is empty, which contains the
+ // value of the `hx-confirm` attribute.
+ if (!e.detail.question) {
+ e.detail.issueRequest(true);
+ } else {
+ showDeletePopup(e);
+ }
+ });
}
function initGlobalShowModal() {