Be able to control the theme outside of the constructor

[kazazor]

At the moment, the rLogin theme is disconnected from the site's theme. The idea is to be able to change the theme as the site theme changes. Otherwise one will have to change the theme on the site and then also on rLogin.

At the moment this is only accessible when initializing rLogin.

So I've set the dark mode to be the default for rLogin (and my site is also at dark mode), if a user changes the site to be in light mode - I have no way of telling rLogin to load at light mode as well, this creating a disconnected experience.

In this case, it would be great also to be able to completely hide the change theme button from rLogin as usually, sites control that and not the wallet modal.

[ilanolkies]

We can add a method rLogin.changeTheme(themeName)

We still want to admit the rLogin to change the theme. The dApp can listen to that change via rLogin.on('themeChanged', (theme) => {}). See https://developers.rsk.co/rif/rlogin/features/. We can add a setting hideThemeSelector: boolean anyway

[kazazor]

That could be good, as long as the Dapp have the option to not show the theme button on rLogin and be able to change the theme during runtime - that works for me :)

[jessgusclark]

Adding the event listener of themeChanged is good but I don't think adding a hideThemeSelector property in the constructor is necessary. Instead this can be handled with plain old CSS: #rlogin-connect-modal #theme-switcher {display: none}.

[kazazor]

@jessgusclark, you mean that the hosting app will have this CSS? Not sure I understood what you meant.

If so I disagree with this approach. That means there is a hardcoded dependency between the app and rLogin. If tomorrow rLogin will change the CSS it will break.

Also for rLogin's development that would be bad as you won't be able to change your CSS without breaking the UI. My (as the dapp developer) interactions with rLogin should be using the standard JS interface so avoid these situations.

[ilanolkies]

IMHO is not necessary to remove rLogin theme button. With the proposal you would be able to do this

const rLogin = new RLogin({ /* ... */ })

const MyApp = () => {
  const [appTheme, setAppTheme] = useState('dark')

  const handleChangeTheme = () => {
    const currentTheme = appTheme
    const nextTheme = currentTheme === 'dark' ? 'light' : 'dark'
    setAppTheme(nextTheme)
    rLogin.changeTheme(nextTheme) // NEW METHOD
  }

  useEffect(() => {
    // removes listener on cleanup
    return rLogin.on('themeChanged', (newTheme) => { // EXISTENT LISTENER
      setAppTheme(newTheme)
    })
  }, [])

  return <ThemeContainer theme={appTheme}>
    My app
    <button onClick={handleChangeTheme}>Change theme</button>
  <ThemeContainer />
}

[kazazor]

It's not necessary needed, depends on the Dapp. In our case it will be confusing as we'll have 2 places to control the theme, which from a UX perspective is wrong.

My guess is the 99% of the dapps with theme control will have the same situation - a Dapp with a theme control will have it on its own ui.

Forcing to show this button here basically treats rLogin like an app on its own. But it's not. It's only a support tool and as such should give me the flexibility I need to embed it in my Dapp with the best UX possible.

IMHO this feature was implemented under the thinking of rLogin as an app of it's own and not a support tool inside an app. I say that because up until this request an app couldn't even change the theme during runtime.. it doesn't make sense changing just the theme of rLogin regardless of the theme of the dapp. IMHO this thinking is wrong for such a tool and should be changed - no need to force Dapp developers to show a button they don't want that was put there separately from the Dapp's theme button