framework icon indicating copy to clipboard operation
framework copied to clipboard

Admin page's Save button won't reset if an error occurs

Open YUCLing opened this issue 1 year ago • 1 comments

Current Behavior

While saving settings, some errors might occur, however, the state of the save button won't reset if an error occurs in saveSettings()

https://github.com/flarum/framework/blob/721e2eae3db2ca59116d17f7b001298d99418953/framework/core/js/src/admin/components/AdminPage.tsx#L412

This makes it not ideal to return a ValidationException in Saving events, nor for other cases where unexpected errors occur since the user must refresh the page to reset the state.

Steps to Reproduce

  1. Go to a settings page of an extension
  2. Click the save button (make sure an error response will be returned from the saving endpoint)

Expected Behavior

Save button is not in loading state after receiving the server response.

Screenshots

image

Environment

  • Flarum version: 1.8.5
  • Website URL: Not suitable
  • Webserver: apache
  • Hosting environment: Not suitable
  • PHP version: 8.3.2
  • Browser: chrome 120

Output of php flarum info

Flarum core: 1.8.5
PHP version: 8.3.2
MySQL version: 11.2.2-MariaDB-1:11.2.2+maria~ubu2204
Loaded extensions: Core, date, libxml, openssl, pcre, sqlite3, zlib, ctype, curl, dom, fileinfo, filter, hash, iconv, json, mbstring, SPL, session, PDO, pdo_sqlite, standard, posix, random, readline, Reflection, Phar, SimpleXML, tokenizer, xml, xmlreader, xmlwriter, mysqlnd, exif, gd, imagick, pdo_mysql, sodium
+--------------------------------+------------+------------------------------------------+
| Flarum Extensions              |            |                                          |
+--------------------------------+------------+------------------------------------------+
| ID                             | Version    | Commit                                   |
+--------------------------------+------------+------------------------------------------+
| flarum-flags                   | v1.8.0     |                                          |
| flarum-approval                | v1.8.1     |                                          |
| flarum-tags                    | v1.8.0     |                                          |
| rhd-ranks                      | dev-main   | 322bf563737f53e8468b61c07ce5ab17e434fc4c |
| rhd-extension                  | dev-main   | cd90d5caf040a27b97c3f2bc366ce5ce7e92aaf5 |
| flarum-suspend                 | v1.8.1     |                                          |
| flarum-subscriptions           | v1.8.0     |                                          |
| flarum-sticky                  | v1.8.0     |                                          |
| flarum-statistics              | v1.8.0     |                                          |
| flarum-mentions                | v1.8.3     |                                          |
| flarum-markdown                | v1.8.0     |                                          |
| flarum-lock                    | v1.8.0     |                                          |
| flarum-likes                   | v1.8.0     |                                          |
| flarum-lang-chinese-simplified | dev-master |                                          |
| flarum-emoji                   | v1.8.0     |                                          |
| flarum-bbcode                  | v1.8.0     |                                          |
+--------------------------------+------------+------------------------------------------+
Base URL: http://127.0.0.1:8181
Installation path: /var/www/web
Queue driver: sync
Session driver: file
Mail driver: mail
Debug mode: off

Possible Solution

Add a catch handler for saveSettings()'s promise and reset the save button state

Additional Context

No response

YUCLing avatar Feb 03 '24 05:02 YUCLing

FYI: Just confirmed this also affects the validators.

YUCLing avatar Feb 04 '24 11:02 YUCLing