ISH-Gruppe / screenario

Screenario - The screen for every scenario
https://screenar.io
GNU General Public License v3.0
2 stars 2 forks source link

Feat/show and hide windows & save layout to local state #27

Closed liam-k closed 2 years ago

liam-k commented 2 years ago

Closes #23

Important: I temporarily removed qrcode-react from package.json in this build because #26. Important: Very breaking changes to WindowManager. It’d be best if you could copy the whole thing into your branches as it is, or merge yours before my pull request so we don’t end up changing it back and forth.

Hooray, we got somewhat usable window management now! Everything works as expected and as a bonus, layouts are saved to the local storage. Only problem left is that my default layout doesn’t work as of now – so when there’s no user-defined layout in the local storage, all windows will be stacked to the left and too small in a very ugly way.

Also, hiding a window and reshowing it will not cause it to reappear in the same spot as before, it’ll just be put wherever there’s space left. This looks ugly in the current state but I think in the final app it’ll actually be pretty handy.

Oh, and another bonus I forgot to mention: We got a button to reset the layout to default. The resetting works, but as I mentioned above, my default layout doesn’t, so it’s more of a "make layout ugly"-button right now. Not sure if we even need it in general, what do you think?

Reviewer's task:

vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
screenario-react ✅ Ready (Inspect) Visit Preview Aug 10, 2022 at 9:31AM (UTC)
s-gbz commented 2 years ago

Amazing mate! Really enjoyable to see the application come to *life. In the spirit of truely ticking of "I didn't see no code smells" I took the liberty to take some minor adjustmens as you will see in the commit history. Also, there've been two minor issues in two commits (the first was a too long title (best practise is 50 characters max and in the second one I adjusted the action taken ("build"))

Hope you don't mind, I naturally made a backup just in case. Merging this now so I can base future branches from this. Speaking of, it's REALLY time to remove CountdownTimer and have it just be Timer. Having to rename it constantly is starting to annoy me :sweat_smile:

s-gbz commented 2 years ago

PS: You don't have to manually close issues, since the "closes" keyword does that for you :ok_hand:

liam-k commented 2 years ago

Thanks for the corrections – I actually directly copied those localstorage functions from the react-grid-layout examples (time constraints & stuff...). Also I thought I had removed the CountdownTimer already, guess I overlooked it.