Allow configuring sidebar default pane action (new pane vs replace)

[neilalexander]

This is somewhat of an attempt to modify the default click behaviour of channels in the sidebar. Today, each channel clicked opens a new pane, which quickly clutters up when dealing with lots of channels. Ideally I'd like to be able to just replace the focused pane instead, allowing me to right-click to open new panes as needed, but to not clutter up the window otherwise.

I figured just changing the default behaviour without configurability is probably a bad thing, so in lieu of that, I've tried to add a new configuration option that lets you choose either NewPane (today's behaviour) or ReplacePane (my proposed behaviour):

dashboard:
  sidebar_default_action: ReplacePane

The fundamental new-vs-replace pane part works but I am struggling with the config side of things. I am not really a native Rustacean (please go easy on me) and I am not really sure of the best place to put this config item or how to pass it down the hierarchy.

It appears that the Dashboard state gets written out to a file, so that means that any config there also gets written into the state file and therefore later changes to the option in config.yaml are ignored. I'd appreciate any pointers on what the best approach here should be or if it should go somewhere else.

[tarkah]

Thanks for the contribution! This looks great and makes sense.

Regarding your question around config vs settings. Our Settings structs are the runtime settings that can be affected by user interactions and are persisted between sessions via persisting that dashboard state to disk.

Since this is a purely config option, we can keep it isolated to config and only worry about loading it and passing it down during view.

You bring up a great point though and we don't have proper separation between our settings and config. We should probably separate these and have proper From implementations and convert them when dashboard / panes are created.

This is probably good to go and we can follow up with a bigger refactor to enforce this separation of concerns. No reason to add that complexity here when this follows the existing setup we're currently using.

[neilalexander]

Thanks for your feedback! I've passed down the config into the dashboard after the state restore which seems to avoid the problem of the state capturing the first config and then never allowing it to change. I've also then propagated to the sidebar via view as suggested.

I've also renamed the option to dashboard.sidebar_default_action as that seems more self-explanatory, although I'm happy to take advice on if you can think of a better name.

Local testing seems to be good, please let me know if there's anything else. 😄

[tarkah]

Change looks good! Just one minor nit, we can avoid duplicating Config data across state since we can just pass it down during view.

[tarkah]

Since we don't plan to ever modify config at runtime, it can be a presentation only item. We can just pass it down during view from our root state and reference it when building the view tree.

[neilalexander]

I see what you mean, how does it look now?

[tarkah]

Looks great! Thanks for this contribution :D I'll follow up next week with a refactor to remove all Settings from config with proper From conversions.

@casperstorm can you just do a quick test of this before we merge? I'm away from a computer for the next few days.

[neilalexander]

Happy to contribute, I'm very much enjoying Halloy. Keep up the great work!

One final thing, do you prefer things to be rebased down into a single commit or shall I leave the history as-is?

[tarkah]

One final thing, do you prefer things to be rebased down into a single commit or shall I leave the history as-is?

Commit history is fine :) The only preference I have is preferring rebasing a feature branch to master vs back-merging when resolving conflicts. But this isn't a hard rule or anything.

[casperstorm]

This is great 👍🏻
Before I merge, please fill out the CHANGELOG with your change, and also update config.yaml with your new configuration.

[neilalexander]

I've rebased on top of your most recent changes and updated the changelog/sample config.