xh / hoist-react

🏗️ ⚛️ The XH Hoist toolkit for React
https://xh.io
Apache License 2.0
24 stars 9 forks source link

New `PersistenceManager` component #2910

Open amcclain opened 2 years ago

amcclain commented 2 years ago

I'd like to propose a new component and model to provide out-of-the-box support for user-managed collections of named, persisted state objects - e.g. grid states or dashboards. While the proposal below would not replace all of our current bespoke in-app implementations of similar features, it would aim to cover the "80%" use case, with room for future development.

User-facing UI

Developer API

TomTirapani commented 2 years ago

Sounds great to me! Will be awesome to build the generic version of this, and start standardising our somewhat haphazard implementations. Particularly like the emphasis on making it easy to ignore w.r.t "you have unsaved changes" - catching and blocking on those actions is always problematic.

A couple of thoughts / questions:

haynesjm42 commented 2 years ago

Have set up something very much like this in a client app, using JsonBlobs for storage - happy to demo/review that anytime

amcclain commented 2 years ago

Thanks for looking over this carefully.

Should we always allow the user to "Save As", even if they don't have local modifications?

Yes, agree - no reason to prevent that, very standard operation to save as before editing.

We need to be careful about built in affordances that claim to reset state, e.g. "Restore Grid Defaults" in the context menu. [...] Perhaps the easiest solution is to provide a way for components to detect when they are being managed by a PersistenceManager, and drop such affordances, relying entirely on the managed state?

Great point. and agree that we need to have a plan here. I think we support hiding the restore option on colChooserModel, and you can pass in a custom grid context menu, but doing that every time would be tiresome. Might cause us to revisit the work we just did around support for custom restoreDefaultsFn - in this case, you might wish to completely intercept/override the default behavior...

Let's make some time next week to discuss in more detail - I will loop back with @lbwexler and get his thoughts on priority. The use case in the client app doesn't look like it's urgent - still important, but we have some time - and while I think this will be a great win for Hoist, we don't want to rush it!