flarum / framework

Simple forum software for building great communities.
http://flarum.org/
6.39k stars 834 forks source link

Admin setting help text disappears after redraw #2728

Closed clarkwinkelmann closed 3 years ago

clarkwinkelmann commented 3 years ago

Bug Report

Current Behavior The help text defined Using ExtensionData.registerSetting() disappears after the first Mithril redraw of the settings page.

Steps to Reproduce

Use following javascript code:

app.initializers.add('my-extension', () => {
    app.extensionData.for('my-extension')
        .registerSetting({
            type: 'switch',
            setting: 'my-setting',
            label: 'A label',
            help: 'A help text',
        });
});

image

Click switch:

image

Poof help text is gone and won't come back until next time the page is opened.

Expected Behavior Help text shouldn't disappear.

Environment

Possible Solution

The problem is here https://github.com/flarum/core/blob/706eaeda4153de130b8530713f00772df7146458/js/src/admin/components/AdminPage.js#L103

The help attribute is deleteed from the object, but since the object is modified by reference, it's gone forever.

The object needs to be cloned before the help attribute is removed.

That delete statement seems very out of place. Why do we remove help and not label ?

Also there's likely a similar issue with className lower in the file where an additional FormControl class will be added on each redraw https://github.com/flarum/core/blob/706eaeda4153de130b8530713f00772df7146458/js/src/admin/components/AdminPage.js#L124

askvortsov1 commented 3 years ago

Something along the lines of:

const { setting, help, ...componentAttrs } = entry;

should fix it, with componentAttrs being passed to the actual input elements instead of the full entry object.

clarkwinkelmann commented 3 years ago

As a note, I used the following workaround in https://github.com/clarkwinkelmann/flarum-ext-emojionearea which is impacted by this issue.

    // Workaround for https://github.com/flarum/core/issues/2728
    override(AdminPage.prototype, 'buildSettingComponent', function (original, entry) {
        return original({...entry});
    });