Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

[Toolbar] How to implement toggle button backed by sharedPrefs? #13710

Closed
data-sync-user opened this issue Aug 12, 2020 · 0 comments
Closed

[Toolbar] How to implement toggle button backed by sharedPrefs? #13710

data-sync-user opened this issue Aug 12, 2020 · 0 comments
Labels
needs:triage Issue needs triage

Comments

@data-sync-user
Copy link

data-sync-user commented Aug 12, 2020

I originally filed #265 to ask a specific question wrt our implementation: this bug is to explain the problem we were trying to generally solve to see if the framework should change to account for it.

We want to have a Toolbar.ActionToggleButton backed by shared preferences, i.e.:

val isButtonEnabled: Boolean
    get() = SharedPrefs.getBoolean(KEY)
    set(value) = SharedPrefs.edit().setBoolean(KEY, value).apply()

fun onAttach() { // or similar
    // Set button state before button is initially drawn
    buttonView.isChecked = isButtonEnabled
    registerOnSharedPrefsCHangeListener(object: SharedPrefsChangeListener { key ->
        if (key == KEY) {
            buttonView.isChecked = isButtonEnabled
        }
    }
}

However, there are two problems in the current implementation: we can't set the state without changing the listener #267 and we're not sure the best way to keep references to the button views #268. As such, we do something funky:

object ToolbarIntegration {

    /**
     * A map that keeps strong references to [OnSharedPreferenceChangeListener]s until the object it
     * manipulates, [BrowserToolbar], is GC'd (i.e. their lifecycles are the same). This is necessary
     * because [SharedPreferences.registerOnSharedPreferenceChangeListener] doesn't keep strong
     * references so someone else, this object, has to.
     */
    private val weakToolbarToSharedPrefListeners = WeakHashMap<BrowserToolbar, OnSharedPreferenceChangeListener>()

    /**
     * Add the components of toolbar.
     */
    fun setup(toolbar: BrowserToolbar,
              navigationStateProvider: BrowserFragment.NavigationStateProvider,
              onToolbarEvent: (result: NavigationEventResult) -> Unit) {
        // ...
        val turboButton = Toolbar.ActionToggleButton(imageResource = R.drawable.turbo_off,
                imageResourceSelected = R.drawable.turbo_on,
                contentDescription = context.getString(R.string.turbo_mode),
                contentDescriptionSelected = context.getString(
                        R.string.onboarding_turbo_mode_button_off),
                selected = Settings.getInstance(toolbar.context).isBlockingEnabled) { isSelected ->
            onToolbarEvent(TURBO.toResult(isSelected))
        }
        toolbar.addBrowserAction(turboButton)

        val sharedPrefsListener = OnSharedPreferenceChangeListener { sharedPreferences, key ->
            if (key == IWebView.TRACKING_PROTECTION_ENABLED_PREF) {
                turboButton.setSelected(sharedPreferences.getBoolean(key, true /* unused */))
            }
        }
        Settings.getInstance(toolbar.context).preferences.registerOnSharedPreferenceChangeListener(sharedPrefsListener)
        weakToolbarToSharedPrefListeners[toolbar] = sharedPrefsListener
    }
}

private fun Toolbar.ActionToggleButton.setSelected(willBeSelected: Boolean) {
    if (isSelected() != willBeSelected) { toggle() }
}

(I actually realize this may be broken because the listener holds a strong reference to the toolbar). This only currently works because the call order is correct #265 for setSelected. What is the recommended way to implement such a button, that is backed by SharedPreferences?

┆Issue is synchronized with this Jira Task

@github-actions github-actions bot added the needs:triage Issue needs triage label Aug 12, 2020
@liuche liuche closed this as completed Aug 12, 2020
@data-sync-user data-sync-user changed the title FNX3-22273 ⁃ [Toolbar] How to implement toggle button backed by sharedPrefs? [Toolbar] How to implement toggle button backed by sharedPrefs? Jun 21, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
needs:triage Issue needs triage
Projects
None yet
Development

No branches or pull requests

2 participants