Skip to content

Commit 33364bf

Browse files
benjiwheelerbenjiwheeler
authored
Merge pull request #5508 from adroitwhiz/stage-layout-refactor
Refactor stage layout
2 parents 609ff1c + 02fa66b commit 33364bf

File tree

7 files changed

+120
-125
lines changed

7 files changed

+120
-125
lines changed

‎src/components/gui/gui.css‎

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,17 @@
3434
}
3535

3636
.editor-wrapper {
37-
flex-basis: 600px;
37+
/*
38+
This is carefully balanced-- the minimum width at which the GUI will be displayed is 1024px.
39+
At that size, the stage pane is 408px wide, with $space of padding to each side.
40+
However, we must also add the border width to the stage pane. All-in-all, the stage pane's final width is
41+
408px + ($space + $stage-standard-border-width * 2) (one border & padding per left/right side).
42+
43+
@todo This is in place to prevent "doubling up" of horizontal scrollbars in narrow windows, but there are likely
44+
much better ways to solve that (e.g. undo #2124, remove this flex-basis entirely). However, they run their own
45+
risks of breaking things, so let's just leave this as-is for the time being.
46+
*/
47+
flex-basis: calc(1024px - 408px - (($space + $stage-standard-border-width) * 2));
3848
flex-grow: 1;
3949
flex-shrink: 0;
4050
position: relative;
@@ -197,21 +207,9 @@
197207
/* pad entire wrapper to the left and right; allow children to fill width */
198208
padding-left: $space;
199209
padding-right: $space;
200-
}
201-
202-
.stage-and-target-wrapper.large {
203-
/* Fix the max width to max large stage size (defined in layout_constants.js) + gutter size */
204-
max-width: calc(480px + calc($space * 2));
205-
}
206-
207-
.stage-and-target-wrapper.large-constrained {
208-
/* Fix the max width to max largeConstrained stage size (defined in layout_constants.js) + gutter size */
209-
max-width: calc(408px + calc($space * 2));
210-
}
211210

212-
.stage-and-target-wrapper.small {
213-
/* Fix the max width to max small stage size (defined in layout_constants.js) + gutter size */
214-
max-width: calc(240px + calc($space * 2));
211+
/* this will only ever be as wide as the stage */
212+
flex-basis: 0;
215213
}
216214

217215
.target-wrapper {

‎src/components/gui/gui.jsx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -336,6 +336,7 @@ const GUIComponent = props => {
336336

337337
<Box className={classNames(styles.stageAndTargetWrapper, styles[stageSize])}>
338338
<StageWrapper
339+
isFullScreen={isFullScreen}
339340
isRendererSupported={isRendererSupported}
340341
isRtl={isRtl}
341342
stageSize={stageSize}

‎src/components/stage-wrapper/stage-wrapper.css‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
@import "../../css/units.css";
22
@import "../../css/colors.css";
3+
@import "../../css/z-index.css";
34

45
.stage-wrapper * {
56
box-sizing: border-box;
@@ -9,3 +10,21 @@
910
/* Hides negative space between edge of rounded corners + container, when selected */
1011
user-select: none;
1112
}
13+
14+
.stage-wrapper.full-screen {
15+
position: fixed;
16+
top: $stage-menu-height;
17+
left: 0;
18+
right: 0;
19+
bottom: 0;
20+
z-index: $z-index-stage-wrapper-overlay;
21+
background-color: $ui-white;
22+
/* spacing between stage and control bar (on the top), or between
23+
stage and window edges (on left/right/bottom) */
24+
padding: $stage-full-screen-stage-padding;
25+
26+
/* this centers the stage */
27+
display: flex;
28+
flex-direction: column;
29+
align-items: center;
30+
}

‎src/components/stage-wrapper/stage-wrapper.jsx‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import PropTypes from 'prop-types';
22
import React from 'react';
3+
import classNames from 'classnames';
34
import VM from 'scratch-vm';
45

56
import Box from '../box/box.jsx';
@@ -22,7 +23,10 @@ const StageWrapperComponent = function (props) {
2223

2324
return (
2425
<Box
25-
className={styles.stageWrapper}
26+
className={classNames(
27+
styles.stageWrapper,
28+
{[styles.fullScreen]: isFullScreen}
29+
)}
2630
dir={isRtl ? 'rtl' : 'ltr'}
2731
>
2832
<Box className={styles.stageMenuWrapper}>

‎src/components/stage/stage.css‎

Lines changed: 22 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,13 @@
2121

2222
/* Make sure border is not included in size calculation */
2323
box-sizing: content-box !important;
24+
25+
/* enforce overflow + reset position of absolutely-positioned children */
26+
position: relative;
27+
}
28+
29+
.stage.full-screen {
30+
border: $stage-full-screen-border-width solid rgb(126, 133, 151);
2431
}
2532

2633
.with-color-picker {
@@ -43,62 +50,30 @@
4350
position: relative;
4451
}
4552

46-
.stage-wrapper-overlay {
47-
position: fixed;
48-
top: $stage-menu-height;
49-
left: 0;
50-
right: 0;
51-
bottom: 0;
52-
z-index: $z-index-stage-wrapper-overlay;
53-
background-color: $ui-white;
54-
/* spacing between stage and control bar (on the top), or between
55-
stage and window edges (on left/right/bottom) */
56-
padding: $stage-full-screen-stage-padding;
57-
}
58-
59-
/* wraps only main content of overlay player, not monitors */
60-
.stage-overlay-content {
61-
outline: none;
62-
margin: auto;
63-
border: $stage-full-screen-border-width solid rgb(126, 133, 151);
64-
padding: 0;
65-
border-radius: $space;
66-
67-
overflow: hidden;
68-
display: flex;
69-
justify-content: center;
70-
}
71-
72-
.stage-overlay-content-border-override {
73-
border: none;
74-
}
75-
76-
/* adjust monitors when stage is standard size:
77-
shift them down and right to compensate for the stage's border */
78-
.stage-wrapper .monitor-wrapper {
53+
/* we want stage overlays to all be positioned in the same spot as the stage, but can't put them inside the border
54+
because we want their overflow to be visible, and the bordered element must have overflow: hidden set so that the
55+
stage doesn't "spill" out from under its rounded corners. instead, shift these over by the border width. */
56+
.stage-overlays {
57+
position: absolute;
7958
top: $stage-standard-border-width;
8059
left: $stage-standard-border-width;
60+
61+
/* the overlay itself should not capture pointer events; only its child elements can do that */
62+
pointer-events: none;
8163
}
8264

83-
/* adjust monitors when stage is full screen:
84-
.stage-wrapper-overlay uses position: fixed instead of relative, so we need
85-
to adjust for the border using a different method */
86-
.stage-wrapper-overlay .monitor-wrapper {
87-
padding-top: calc($stage-full-screen-stage-padding + $stage-full-screen-border-width);
88-
padding-bottom: calc($stage-full-screen-stage-padding + $stage-full-screen-border-width);
65+
.stage-overlays.full-screen {
66+
top: $stage-full-screen-border-width;
67+
left: $stage-full-screen-border-width;
8968
}
9069

9170
.monitor-wrapper,
92-
.color-picker-wrapper,
9371
.frame-wrapper,
9472
.green-flag-overlay-wrapper {
9573
position: absolute;
9674
top: 0;
9775
left: 0;
98-
width: 100%;
99-
height: 100%;
10076
pointer-events: none;
101-
overflow: hidden;
10277
}
10378

10479
.dragging-sprite {
@@ -107,14 +82,15 @@ to adjust for the border using a different method */
10782
left: 0;
10883
z-index: $z-index-dragging-sprite;
10984
filter: drop-shadow(5px 5px 5px $ui-black-transparent);
110-
}
85+
}
11186

11287
.stage-bottom-wrapper {
11388
position: absolute;
11489
display: flex;
11590
flex-direction: column;
11691
justify-content: flex-end;
11792
top: 0;
93+
left: 0;
11894
overflow: hidden;
11995
pointer-events: none;
12096
}
@@ -141,6 +117,8 @@ to adjust for the border using a different method */
141117
}
142118

143119
.green-flag-overlay-wrapper {
120+
width: 100%;
121+
height: 100%;
144122
display: flex;
145123
justify-content: center;
146124
align-items: center;

‎src/components/stage/stage.jsx‎

Lines changed: 60 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -35,23 +35,17 @@ const StageComponent = props => {
3535
const stageDimensions = getStageDimensions(stageSize, isFullScreen);
3636

3737
return (
38-
<div>
38+
<React.Fragment>
3939
<Box
40-
className={classNames({
41-
[styles.stageWrapper]: !isFullScreen,
42-
[styles.stageWrapperOverlay]: isFullScreen,
43-
[styles.withColorPicker]: !isFullScreen && isColorPicking
44-
})}
45-
style={{
46-
minHeight: stageDimensions.height,
47-
minWidth: stageDimensions.width
48-
}}
40+
className={classNames(
41+
styles.stageWrapper,
42+
{[styles.withColorPicker]: !isFullScreen && isColorPicking})}
4943
onDoubleClick={onDoubleClick}
5044
>
5145
<Box
5246
className={classNames(
5347
styles.stage,
54-
{[styles.stageOverlayContent]: isFullScreen}
48+
{[styles.fullScreen]: isFullScreen}
5549
)}
5650
style={{
5751
height: stageDimensions.height,
@@ -66,18 +60,61 @@ const StageComponent = props => {
6660
}}
6761
{...boxProps}
6862
/>
63+
<Box className={styles.monitorWrapper}>
64+
<MonitorList
65+
draggable={useEditorDragStyle}
66+
stageSize={stageDimensions}
67+
/>
68+
</Box>
69+
<Box className={styles.frameWrapper}>
70+
<TargetHighlight
71+
className={styles.frame}
72+
stageHeight={stageDimensions.height}
73+
stageWidth={stageDimensions.width}
74+
/>
75+
</Box>
76+
{isColorPicking && colorInfo ? (
77+
<Loupe colorInfo={colorInfo} />
78+
) : null}
6979
</Box>
70-
<Box className={styles.monitorWrapper}>
71-
<MonitorList
72-
draggable={useEditorDragStyle}
73-
stageSize={stageDimensions}
74-
/>
75-
</Box>
76-
<Box className={styles.frameWrapper}>
77-
<TargetHighlight
78-
className={styles.frame}
79-
stageHeight={stageDimensions.height}
80-
stageWidth={stageDimensions.width}
80+
81+
{/* `stageOverlays` is for items that should *not* have their overflow contained within the stage */}
82+
<Box
83+
className={classNames(
84+
styles.stageOverlays,
85+
{[styles.fullScreen]: isFullScreen}
86+
)}
87+
>
88+
<div
89+
className={styles.stageBottomWrapper}
90+
style={{
91+
width: stageDimensions.width,
92+
height: stageDimensions.height
93+
}}
94+
>
95+
{micIndicator ? (
96+
<MicIndicator
97+
className={styles.micIndicator}
98+
stageSize={stageDimensions}
99+
/>
100+
) : null}
101+
{question === null ? null : (
102+
<div
103+
className={styles.questionWrapper}
104+
style={{width: stageDimensions.width}}
105+
>
106+
<Question
107+
question={question}
108+
onQuestionAnswered={onQuestionAnswered}
109+
/>
110+
</div>
111+
)}
112+
</div>
113+
<canvas
114+
className={styles.draggingSprite}
115+
height={0}
116+
ref={dragRef}
117+
width={0}
81118
/>
82119
</Box>
83120
{isStarted ? null : (
@@ -86,52 +123,14 @@ const StageComponent = props => {
86123
wrapperClass={styles.greenFlagOverlayWrapper}
87124
/>
88125
)}
89-
{isColorPicking && colorInfo ? (
90-
<Box className={styles.colorPickerWrapper}>
91-
<Loupe colorInfo={colorInfo} />
92-
</Box>
93-
) : null}
94-
<div
95-
className={styles.stageBottomWrapper}
96-
style={{
97-
width: stageDimensions.width,
98-
height: stageDimensions.height,
99-
left: '50%',
100-
marginLeft: stageDimensions.width * -0.5
101-
}}
102-
>
103-
{micIndicator ? (
104-
<MicIndicator
105-
className={styles.micIndicator}
106-
stageSize={stageDimensions}
107-
/>
108-
) : null}
109-
{question === null ? null : (
110-
<div
111-
className={styles.questionWrapper}
112-
style={{width: stageDimensions.width}}
113-
>
114-
<Question
115-
question={question}
116-
onQuestionAnswered={onQuestionAnswered}
117-
/>
118-
</div>
119-
)}
120-
</div>
121-
<canvas
122-
className={styles.draggingSprite}
123-
height={0}
124-
ref={dragRef}
125-
width={0}
126-
/>
127126
</Box>
128127
{isColorPicking ? (
129128
<Box
130129
className={styles.colorPickerBackground}
131130
onClick={onDeactivateColorPicker}
132131
/>
133132
) : null}
134-
</div>
133+
</React.Fragment>
135134
);
136135
};
137136
StageComponent.propTypes = {

‎src/containers/green-flag-overlay.jsx‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,8 +21,6 @@ class GreenFlagOverlay extends React.Component {
2121
}
2222

2323
render () {
24-
if (this.props.isStarted) return null;
25-
2624
return (
2725
<Box
2826
className={this.props.wrapperClass}
@@ -42,13 +40,11 @@ class GreenFlagOverlay extends React.Component {
4240

4341
GreenFlagOverlay.propTypes = {
4442
className: PropTypes.string,
45-
isStarted: PropTypes.bool,
4643
vm: PropTypes.instanceOf(VM),
4744
wrapperClass: PropTypes.string
4845
};
4946

5047
const mapStateToProps = state => ({
51-
isStarted: state.scratchGui.vmStatus.started,
5248
vm: state.scratchGui.vm
5349
});
5450

0 commit comments

Comments
 (0)