homeroom-live / homeroom

Live streaming for education.
0 stars 0 forks source link

withEditable HOC #28

Open hoodsy opened 6 years ago

hoodsy commented 6 years ago

@maticzav Not sure how exactly to wire this up to work with mutations – I imagine the HOC receives handleSave or something like that, so that each field can be updated in place.

https://github.com/homeroom-live/homeroom/blob/e1a7d44b35144f001abc75efeac54853afc0754f/www/sections/dashboard/classInformation.js#L159-L162

Demo: https://homeroom-live.slack.com/files/U8SBTK67M/FBNKDHE3H/witheditable.mov

maticzav commented 6 years ago

I think there are two options, but I can tell you upfront that I lean towards the second one. πŸ˜‚

So, the saving process means taking all the current values and running Apollo mutation which updates the saved values of the fields. I imagine we can achieve this most easily if we use getInitialProps function and query everything once customer lands on our page, and copy everything to the internal state.

Once the customer changes values, we only update the component state and, in the end, run a mutation which updates values on the server as well.

My thoughts on how we should trigger the update; 1) Use debounce function and save automatically after some time. 2) Display a save button and save everything once it's pressed or the customer submits the form.

hoodsy commented 6 years ago

If the user is editing text inline, the expectation is that their changes will be saved after each edit... If we go save button, I think a typical form makes more sense.

That being said, what if we had the same save trigger wired up to each EditableComponent? Each time one of them is changed and the user hits enter, clicks the confirm checkmark, or clicks off the input, then the save is registered

maticzav commented 6 years ago

Whoa yea that could be cool! Have you seen onBlur property yet? I think we could do a ful mutation every time something changes and make it visible that it changed - maybe we should disable everything when it’s updating and show indicator?

I think we can wire everytihng up in a Mutation component and just include the trigger inside every component

hoodsy commented 6 years ago

Yes this should work!

Do we even need to disable changes while loading?

On Wed, Jul 11, 2018, 1:17 AM Matic Zavadlal notifications@github.com wrote:

Whoa yea that could be cool! Have you seen onBlur property yet? I think we could do a ful mutation every time something changes and make it visible that it changed - maybe we should disable everything when it’s updating and show indicator?

I think we can wire everytihng up in a Mutation component and just include the trigger inside every component

β€” You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/homeroom-live/homeroom/issues/28#issuecomment-404084871, or mute the thread https://github.com/notifications/unsubscribe-auth/AEQruh-3jK17P-UcEzYBYaivq2JvJ2F2ks5uFbSOgaJpZM4VIhSN .

maticzav commented 6 years ago

I would say it depends; if we want to make it feel responsive then no. I think it might be good to keep states in sync this way. But now that I think of it might be unnecessary. πŸ‘Œ

hoodsy commented 6 years ago

Needs to be wired up but won't be used in this release.