Closed ksraj123 closed 4 years ago
Review of Changes Made
Total 7 files changed. Below is the description of changes made to each file.
Sugarizer.js - inFullScreenMode
property added to state of Sugarizer component, toggleFullScreen
method added to handle click events on the concerned buttons. These are passed as props to Navbar
and Main
components.
Router.js - Main
component turned from a functional component to class component. zoom
property added to the state. componentDidUpdate
added inside which height of the Main
component is found and resizeBoard
function is called which determines the the value of zoom
using logic similar to what is used the qr code activity and updates the state.
translations.js - Strings added which will be displayed in the tooltips of fullscreen and unfullscreen button.
Navbar.js - FullScreen/UnFullScreen Button Added to the Navbar Component. Conditional logic added to render either the navbar or the unfullscreen button depending upon if it is in fullscreen mode right now which is passed in as a prop from Sugarizer component and is a part of its state.
Navbar.css - styles added for fullscreen and unfullscreen button, the unfullscreen button is made to align over the fullscreen button and right to the stop button. The fullscreen button stays visible in the toolbar even if width of screen is less than ideal.
fullscreen.svg / unfullscreen.svg - two new files added in the icons directory, icons which were already being used in the qr code activity are reused.
@thingthing could you review this fix and compare to https://github.com/llaske/ExerciserReact/pull/93?
Note that the Fullscreen button should be between the Stop button and the Help. The Stop button must be to the right of the screen.
@llaske Suggested Changes made
I fixed no space below the submit button in un-fullscreen mode issue only for GroupAssignmentForm
in #95 because the issue was noticeable in only this container on desktop. However, it was apparent in other containers as well on mobile size. It has now been fixed in all containers.
Improved zooming functionality to better use available space
Before
Now
It don't work on my side. In Sugarizer, I've got the following issue in the browser console:
react-dom.production.min.js:187 TypeError: Object(...) is not a function
at n (index.js:20)
at v (Router.js:29)
at ye (react-dom.production.min.js:172)
at je (react-dom.production.min.js:201)
at Le (react-dom.production.min.js:202)
at ct (react-dom.production.min.js:211)
at at (react-dom.production.min.js:210)
at ot (react-dom.production.min.js:210)
at tt (react-dom.production.min.js:208)
at _e (react-dom.production.min.js:206)
ke @ react-dom.production.min.js:187
Se.e.callback @ react-dom.production.min.js:194
ie @ react-dom.production.min.js:143
ee @ react-dom.production.min.js:144
Bt @ react-dom.production.min.js:219
ct @ react-dom.production.min.js:211
at @ react-dom.production.min.js:210
ot @ react-dom.production.min.js:210
tt @ react-dom.production.min.js:208
_e @ react-dom.production.min.js:206
ht @ react-dom.production.min.js:222
dt @ react-dom.production.min.js:223
pt.render @ react-dom.production.min.js:230
(anonymous) @ react-dom.production.min.js:234
It @ react-dom.production.min.js:221
Rt @ react-dom.production.min.js:234
render @ react-dom.production.min.js:236
(anonymous) @ index.js:10
i @ bootstrap 92665c002444647e85b4:19
(anonymous) @ main.d23fae36.js:16270
i @ bootstrap 92665c002444647e85b4:19
(anonymous) @ bootstrap 92665c002444647e85b4:62
(anonymous) @ bootstrap 92665c002444647e85b4:62
index.js:20 Uncaught TypeError: Object(...) is not a function
at n (index.js:20)
at v (Router.js:29)
at ye (react-dom.production.min.js:172)
at je (react-dom.production.min.js:201)
at Le (react-dom.production.min.js:202)
at ct (react-dom.production.min.js:211)
at at (react-dom.production.min.js:210)
at ot (react-dom.production.min.js:210)
at tt (react-dom.production.min.js:208)
at _e (react-dom.production.min.js:206)
n @ index.js:20
v @ Router.js:29
ye @ react-dom.production.min.js:172
je @ react-dom.production.min.js:201
Le @ react-dom.production.min.js:202
ct @ react-dom.production.min.js:211
at @ react-dom.production.min.js:210
ot @ react-dom.production.min.js:210
tt @ react-dom.production.min.js:208
_e @ react-dom.production.min.js:206
ht @ react-dom.production.min.js:222
dt @ react-dom.production.min.js:223
pt.render @ react-dom.production.min.js:230
(anonymous) @ react-dom.production.min.js:234
It @ react-dom.production.min.js:221
Rt @ react-dom.production.min.js:234
render @ react-dom.production.min.js:236
(anonymous) @ index.js:10
i @ bootstrap 92665c002444647e85b4:19
(anonymous) @ main.d23fae36.js:16270
i @ bootstrap 92665c002444647e85b4:19
(anonymous) @ bootstrap 92665c002444647e85b4:62
(anonymous) @ bootstrap 92665c002444647e85b4:62
In Debug mode I've got the error:
useDimensions
package was added to determine the height of component which uses hooks and is a hook itself but hooks were introduced in react v16.8.0
while this repo uses v16.4.0
. I believe this is what is causing this issue. I think there are no breaking changes between v16.4.0
and v16.8.0
as per the react change logs (https://github.com/facebook/react/blob/master/CHANGELOG.md). I could reproduce this issue in a fresh environment.
@llaske please review
I can run it now. BTW your fix cause an issue in drag&drop of items. See for example in Roman numeral sample. Here's your fix:
Here's the dev.sugarizer.org version:
@llaske Please review
Here is a list of changes made -
Changes made to names of a few classes and ids to better follow react conventions.
Made more improvements to the zooming functionality.
added logic to make changes to each component's styling in fullscreen mode to better utilitze the extra space.
The reason for the issue with drag n drop was because transform scale
scales only the displayed output and does not modify the box sizing of the element due to which position of element was getting messed up when it was being dragged while zoom
does exactly what was needed. css zoom
property is supported by all prominent browsers except firefox. However logic is added for the component's styling to change to make efficient use of the extra space in fullscreen mode. So I think the fullscreen mode serves its purpose on firefox as well.
Here is a walkthrough on firefox -
Thanks the issue is fixed.
BTW I've found another issue. When the fullscreen button is clicked then unfullscreen button is clicked, the activity palette, the presence palette and the stop button no longer work.
@llaske issue fixed, please review.
@llaske Is there anything more expected from this PR? Requesting Review.
Full screen / Unfullscreen button added with resizing functionality.
Fixes #92 and https://github.com/llaske/sugarizer/issues/602
on small screen size 500 x 300
resizing in action - size of board increases in fullscreen mode
The dark flashes are due to the screen recording software. Sorry for that.
desktop size screen