Refactor in-context state to feature file

[shakyShane]

Reviewer:
Asana:

Description

@alistairjcbrown This is my 100% my issue for not seeing this earlier, but I was debugging some things recently I noticed that the purpose of Settings has not been communicated correctly in the past, something I will address.

For now, the short version is this: can we just create a feature-file for this work instead, which will allow you to hold your own internal state and not need to interact with settings as much (only for the true purpose, which was feature flags etc)

Please read through the comments and let me know if anything isn't clear, but the short version is this:

• Settings was designed as a cross-platform way to retrieve global settings/feature toggles etc, we shouldn't expand it's use to feature-related state
• When we do add new features, we can use the composition + dependency injection techniques instead, it's the exact same code if you look, but now the feature is self contained. It also then gives you a little container (the class) in which you can have your own internal state without this feature touching too many parts of the codebase.
• If you look at what I've removed from Settings and extension.transport.js I'm sure you'll be happy 😎

export class InContextSignup {
    // your own internal state, instead of expanding the scope of `Settings`
    permanentlyDismissed = false
    initiallyDismissed = false

    /**
     * Pass the device as a constructor param, and then do everything as before
     * @param {import("./DeviceInterface/InterfacePrototype").default} device
     */
    constructor(device) {
        this.device = device
    }

    async init() { 
       // etc 
    } 
}

// then, you can do the composition + dependency injection like this:
class ExtensionInterface extends InterfacePrototype {
    inContextSignup = new InContextSignup(this)
}

Steps to test

Can you run this locally and see if it works? I just put it together as a POC (I've not changed any functionality of course) but it would be good to get this resolved before your feature is rolled out :)

[github-actions]

Note: Password rules outdated

inntopia.travel not present in current
	rules: minlength: 7; maxlength: 19; required: digit; allowed: upper,lower,[-];

You can update the rules with the following command

cd packages/password && npm run rules:update

Once you've updated the rules, re-run the build from the root with npm run build and then commit all changes.

[shakyShane]

we can prevent the base InterfacePrototype from growing in size for every feature we add by using composition like this and dependency injection so that the feature can call back to the device.

[shakyShane]

both of these methods are only relevant to this feature, so move these into it

[shakyShane]

no need to refresh global settings - anything this 'feature' needs can be done in 'setupAutofill'

[shakyShane]

moved to the base, since it's truly generic/reusable

[shakyShane]

if any features need 'startup' data, fetch it here inside 'setupAutofill'

[shakyShane]

null because not all devices support this yet, and TS helps by forcing us to check for null

[shakyShane]

instead of adding to Settings, why not just add some specific data here?

[shakyShane]

this is the logic that was mixed between the extension transport and the Settings, we can just make the call directly here and store the data ourselves without it expanding the scope of the Device or Settings

[shakyShane]

This verbose pattern of fetching and verifying the data was for the global configuration, feature toggles and available input types since they affect ALL features one way or another.

I can see why you added these here, and we probably need to update the docs/guides to be clearer, but we shouldn't be expanding Settings to include any new state from individual features. :)

[shakyShane]

As mentioned above, this appears to be user-state related and not actually relevant to 'global' settings, so it's moved into the feature file

[alistairjcbrown]

I added a branch with integration tests, and this branch passed 🎉 (with the typo fix applied) -- #254

[shakyShane]

@alistairjcbrown feel free to just take over this PR/branch and rebase against the integration tests etc.

Basically I'll leave you to apply both this + integration tests, just ping when ready and I can approve stuff :)

[alistairjcbrown]

LGTM