View-Only Sketches and Major Main/Editor Refactoring #219
Conversation
…button components to common, remove unnecessary button imports
…utput banners to their components
removing merge conflicts
…torContainer from Main
ahhhhh this is all so jank
fixes weird bottom bar, makes styling on header more consistent. maybe a less jarring white makes this look less bad
forces them to render the logged-in panel
moved all the getting programs/error handling to ViewOnly itself; fixes problem that render functions cannot have async responses possible TODO: "sketch not found" page
The bug: Open a sketch in view-only mode. Go to the sketches page and try to download your most recently viewed sketch. The sketch title and extension are as expected, but the code is the code from the view-only sketch. This fixes the bug by saving the original code for the most recently viewed program, and restoring it before the component unmounts.
|
Just for transparency, @jamieliu386 discovered a bug where we corrupt the Redux state while viewing a view-only program when logged-in. Currently working on an (elegant) fix right now! |
this is also a bandaid fix
View-Only: Fixes Redux State Corruption
There was a problem hiding this comment.
Hey all, great work on this! View-only mode will be great and the refactor is looking fresh 👀
Sorry for the radio silence - I am just now wading through the last few major changes I haven't looked at yet, and gradually testing. To be specific, I am yet to look at the brand new files in depth (EditorAndOutput, ViewOnly), and still want to fool around with user input that might break things. Should have those done by tomorrow.
Again, great work all! Glad to see this all rolling this out.
There was a problem hiding this comment.
Finished parsing through the last few files and played around with the view only editor more. The only thing I stumbled on was that the view only editor does not load the set theme from cookies an user's cookies:
- Log out
- Load a sketch in view only, then change to light theme
- Refresh. Theme does not persist.
Don't know if this was intentional or not - I think that dark theme is a sensible default and there really isn't much sense in attaching a cookie to a user who is likely just viewing it as a once-off. Still, thought that I should bring it up since we do set cookies on theme change from within the view only editor.
Other than this, LGTM!
Ah, that seems like a bug. If you have the time to take a look at that, that'd be great, but if not, I can probably fix it tomorrow! |
|
Jeez, I think this is going to live in some sort of infamy. |
Wow, it's a relief to finally write out this PR! In sum, this is the PR that implements all the necessary work for a View-Only page for each sketch (which works whether or not the user is logged in). It required a lot of code motion and refactoring, and likely has lots of side effects (so I'd prefer some semi-rigorous testing and multiple reviewers). Many thanks to @jamieliu386 and @emmyc for the solid (and gruelling) work they put into this feature!
(this supersedes #103)
Here's a semi-detailed list of what we did:
Editor,Output, andSplitPaneout ofMainto new componentEditorAndOutput; similarly moves requisite props/state andCodeDownloaderwork. necessary to remove code duplicationViewOnlyand accompanyingViewOnlyContainerfor view-only programs. this component manages fetching the program from the pid, handling errors, and passing down the relevant props toEditorAndOutput/p/:pidfor users to visit view-only sketches - directs toViewOnlythis.props.viewOnly,this.props.vLanguagetoOutput, to process props differently depending on program stateEditorbanner behaviour (e.g. Dropdown vs. text, save button) dependent on view-only modeProfilePanelbehaviour to have Create User and Login buttons when the user isn't logged in; maintains themeing capabilities(also, we should probably squash merge this, but I'm not sure how useful the commit history actually might be!)