Rewrite the main menu state management to use Redux

[skateman]

The main menu has a lot of shared state among sub-components and it's getting hard to follow which state variable or setter should be (or should not be) passed to the child components. Also some pieces of the state could be a nice to have if they would be available globally.

So I am going to rewrite the main menu components to use Redux for their state management.

@himdel please let me know if this is a bad idea 😆

[himdel]

Yeah, it is a bad idea :).

There is only one piece of menu state that would make sense in redux .. that's the constant array of menuitems.
The rest is just callbacks (which can't live in redux), and bools about what's hidden. I don't see much of a point..

EDIT: that said, looking at some of the a11y PRs, yeah, I see what you mean :) We may be able to simplify by moving things into separate subcomponents too. (But if redux helps, go for it, I just don't see how.)

[skateman]

Okay, if not redux, I could just convert the menu state into single-object that we could store in a React context with its setState and we could pull out its members in any underlying component. This way probably the param passing would be more followable...

[himdel]

Maybe I don't understand what kind of params you're talking about.

It's probably cleaner to only pass the relevant props to the relevant menu subcomponents,
and be explicit in what we're passing.

What's the actual problem you're solving?

[skateman]

We are passing state variables on 4+ levels among components, mapping functions, then components, then mapping functions, conditionals and components again. This is everything but followable, I'd be for a contextual store from where any component could pull out what it needs...

[himdel]

Can you be specific please? As far as I can tell, the only state being passed to all the menu components is:

• appearExpanded

As far as I can tell, the bits of state passed to more than 1 component are:

• applianceName (2)
• currentUser (2)
• menu (2)
• hideSecondary (2)
• appearExpanded (5)

So,applianceName and currentName - feel free to merge them in a single object if it helps,
menu is static, probably don't merge if we don't want full re-renders,
hideSecondary can't live in redux, it's a callback.

So ... you want redux for appearExpanded?

[miq-bot]

This issue has been automatically marked as stale because it has not been updated for at least 3 months.

If you can still reproduce this issue on the current release or on master, please reply with all of the information you have about it in order to keep the issue open.

Thank you for all your contributions! More information about the ManageIQ triage process can be found in the triage process documentation.