League of Extraordinary FoundryVTT Developers•4y ago

Dors project

To be clear, I'm very open to the feedback that I'm doing something stupid here, hah. This is my first Foundry work and first work of any size in js so doing dumb things is a daily occurrence. That said, I think some of the gap in understanding here is a lack of context about what the page is doing, which makes sense because I haven't explained it - I was looking for a pretty narrow solution. Let me explain some of what the form is doing and you can tell me if it still sounds like I'm doing a Dumb Thing. (as an aside, this is probably a good use case for threading, hah)

11 Replies

Calego•4y ago

@dor thread acheived 😛

DorOP•4y ago

Siiiick 🙂 Okay so: The module is for managing fonts in Foundry. The only means to interact with the module is through a custom settings menu, after that it works under the hood - think 'Dice So Nice' as an example. The form has settings for a number of things - enabling/disabling included font packs, changing how fonts are sorted in dropdowns, specifying which specific fonts should be enabled/disabled at a given time, etc. Some of these settings have to react to each other, hence the submitOnChange. For example, if you enable a new font pack I change the associated setting, re-render (picking up the change in getData), and now that pack is seamlessly available on another tab when you're selecting which individual fonts from that pack to enable or disable at any given time. submitOnChange works great for all of this. The specific problem I ran into here is on the form tab used for adding fonts from the Google Fonts API to Foundry. This will be easier if I grab a couple of screenshots, let me do that real quick... Disclaimer: None of the CSS/text/formatting here is final, hah.

DorOP•4y ago

DorOP•4y ago

So there's the default state of the tab. The user enters the name of the font they want to add from Google Fonts, and presses 'Check Font.' Previously, this button was called 'Add Font' and it added the font immediately, which did go through _updateObject, set the relevant setting, and reload the form. But this was clunky. It meant that the user had to add this font, then hop over to another tab to see what it looked like in context, remove if if they didn't like it, then come back here to add some other font. Not great UX. There's also a lot of checks that need to be run on the user submission before adding it to the appropriate setting - is it gibberish, does the font even exist on Google Fonts, etc. So now we have Check Font, which is the button NOT intended to submit. I listen for the click via activateListeners and do a few things 1. Validate the submission is real and exists 2. Load the font into the browser on a one-off basis 3. Change some CSS classes to both reveal and style some preview text and give the user two new buttons: - 'Add Font' to actually persist the font to settings (which DOES submit, run through _updateData, and render) 'Cancel , which just renders the page without making settings changes, setting us back to the initial state and clearing the user input.

DorOP•4y ago

Note that 'Check Font' does not do any persistence at all, it's all just page transformation, hence not wanting it to submit. Alright I think that's probably enough.

Calego•4y ago

woof I see and understand your conundrum My suggestion for "simplest solution" would be to not do this inside this FormApplication, but instead open a Dialog or new FormApplication whose purpose is to search for fonts. Separating those concerns and flows will keep your code simpler at the cost of this delightfully clean UX

Calego•4y ago

@dor thinking about this, bug reporter does something akin to this with its search for duplicate issues. Instead of requiring a button click to search, we do that automatically. It looks like we ended up overriding _onChangeInput to specifically exclude one input: https://github.com/League-of-Foundry-Developers/bug-reporter/blob/master/Module/main.js#L287

GitHub

bug-reporter/main.js at master · League-of-Foundry-Developers/bug-r...

Contribute to League-of-Foundry-Developers/bug-reporter development by creating an account on GitHub.

DorOP•4y ago

Interesting. I'll spend some more time looking at that, that might be a possible solution. I'd love to avoid adding additional forms/dialogs if I can, both from a layout complexity and UX prespective. @calego So, for various reasons event.preventDefault never would have worked, but your model from BugReporter did. YMMV on whether it is a better solution than separating into another form:

    // Override _onChangeInput to Avoid submitting 'Add Font' input box based on focus change.
    // Will still be submitted by 'Add Font' button
    _onChangeInput(event) {
        let targetName = event.target.name;
        if (targetName === 'gmAddFontInput') {
        }
    }

    // Override _onChangeInput to Avoid submitting 'Add Font' input box based on focus change.
    // Will still be submitted by 'Add Font' button
    _onChangeInput(event) {
        let targetName = event.target.name;
        if (targetName === 'gmAddFontInput') {
        }
    }

Calego•4y ago

Nice! Glad that worked for you! Do note that you've strayed from the "intended API design" of FormApplications and will need to be in guard for that API to change in updates A little hacking never hurt nobody :p said the leader of a community of hackers

DorOP•4y ago

Haha Appreciate the input and assist. Threw the takeaway back in the main channel for posterity, feel free to archive whenever.

Gaming

Programming

Dors project