snapcrunch / electron-preferences

A simple, consistent interface for managing user preferences within an Electron application.
MIT License
122 stars 31 forks source link

Reset optional file field #156

Open macfire opened 2 years ago

macfire commented 2 years ago

Would like option to display a "reset button" on file field when input is optional.

My current implementation: Add button field after the file field, then attach click event to set file field value to null.

Example:


const prefs = new ElectronPreferences {
    (sections -> form -> groups ->) fields: [
        {
            label: 'Background image (optional)',
            key: 'backgroundImage',
             type: 'file',
            filters: [
               {name: 'All Images', extensions: ['jpg', 'jpeg', 'png', 'svg']},
            ],
             multiSelections: false,
        },
        {
            label: '',
            key: 'removeBackgroundImageButton',
            type: 'button',
            buttonLabel: 'Remove background image'
         },
       ]
}

prefs('click', key => {
    if (key === 'removeBackgroundImageButton') {
        preferences.value('theme.backgroundImage', null);
        preferences.save();
    }
})

Issue

The preferences JSON is updated/saved correctly, and main window updates. However, I don't know how to update the Preferences window render. The file field still shows the previously selected file until window is closed and re-opened. Any suggestions welcomed. Thanks.

pvrobays commented 2 years ago

Hi @macfire Thanks for reporting this issue.

In fact I like the idea of adding a 'reset' button of some kind when the file or folder type is indicated as 'optional'. I would add a new boolean option 'optional' to the file and folder type. When this is true, we can show a new 'reset' button.

The workaround you try is not that ideal indeed as there's no way to say to the UI to refresh at any point. The preferences window will just render the initial preferences and refresh while the user interacts with the preference window. One thing you could do is programmatically close and reopen the preference window, but this will just open the first tab. Hoping your file type lives there...

If you'd like to contribute to this repo, please go ahead and open a PR with the above suggested solution :)

lacymorrow commented 2 years ago

Definitely support this idea, I currently have to add extra preferences to "Enable" a customization because there's no simple way to reset (especially the color-picker).

I also had to restrict the keys "Escape"/"Backspace"/"delete" from the accelerator to be used to reset the input. After this is added it may be worth going back to allow those keys to be used normally

pvrobays commented 2 years ago

Good point. We should review the different types and check if an 'optional' boolean is applicable per type. My proposal:

lacymorrow commented 2 years ago

I'm going to make an argument for dropdown (select), radio, and slider, to have reset buttons.

It's not necessarily an issue as the program can offer a "not selected" option (at least for select and radio, but there are cases where a <select> or [type="radio"] input is loaded without a value selected. Basically, an "untouched" or "unset" state. It would be valuable for the slider as well.

For my app, I have settings that are not applicable until the user has set them, so this would be helpful in those scenarios.

macfire commented 2 years ago

@pvrobays , @lacymorrow

I've cloned this repository, and updated the file field with a reset button as follows.

In components/fields/file/index.jsx


constructor(props) {

    super(props);

    this.choose = this.choose.bind(this);

    this.reset = this.reset.bind(this);
}

render() {

....

    const btResetLabel = 'Reset';

    return (
        <div className={`field field-file key-${this.field.key}`}>
            <div className="field-label" aria-label={ label }>{label}</div>
            <div className="value" onClick={this.choose}>
            {multiSelections ? 'Files' : 'File'}:&nbsp;
            {
                value
                ? (
                    multiSelections || value.length > 1
                        ? <ul>{value.map((v, i) => <li key={i}>{v}</li>)}</ul>
                        : value[0]
                    )
                : 'None'
            }
            </div>
            <button className="bt" onClick={this.choose} aria-label={ btLabel }>
                {btLabel}
            </button>
            <button className="bt bt-reset" onClick={this.reset} aria-label={ btResetLabel }>
                {btResetLabel}
            </button>
            {help && <span className="help">{help}</span>}
        </div>
    );

}

// new method
reset() {

    this.onChange();

}

Note: I created a default label at variable btResetLabel. I'm not sure if you would want that as configurable?

pvrobays commented 2 years ago

Hi @macfire Thanks for the contribution! Sorry for the late response. You can submit a PR to merge it into the official development branch. That way it's easier to give you feedback.

The default variable with btResetLabel would indeed be nice if it could be configurable. You can indeed just name it 'Reset' in case it's not configured by the user.

A few remarks I noticed already:

lacymorrow commented 2 years ago

@pvrobays exactly, reset to default value if there is one, and null if not.