From ba3ff9d1f3f00596d1b24b2a566965cc525a5cd7 Mon Sep 17 00:00:00 2001
From: Josiah Neuberger <josiah@wikimylife.org>
Date: Wed, 13 Dec 2017 22:01:09 -0500
Subject: [PATCH] fixed some minor code smell problems
1. Removed duplication of code in stage component to avoid future
potential divergence. Now just selectively applying styling based
on states of zoomed, colorPicking, and question.
2. Fixed bottom border getting chopped in zoomed mode by adding margin
and adjusting height/width accordingly.
3. Removed shouldComponentUpdate from stageHeader as suggested from code
review. Added ignore lint warning suggesting change to stateless function.
---
src/components/stage/stage.css | 6 +++
src/components/stage/stage.jsx | 83 ++++++++++++++++-----------------
src/containers/stage-header.jsx | 6 +--
3 files changed, 46 insertions(+), 49 deletions(-)
diff --git a/src/components/stage/stage.css b/src/components/stage/stage.css
index 425ebde1f..8188751d1 100644
--- a/src/components/stage/stage.css
+++ b/src/components/stage/stage.css
@@ -51,6 +51,8 @@
margin: auto;
border: 3px solid rgb(126, 133, 151);
padding: 0;
+ margin-top: 3px;
+ margin-bottom: 3px;
border-radius: $space;
overflow: hidden;
@@ -59,6 +61,10 @@
justify-content: center;
}
+.stage-overlay-content-border-override {
+ border: none;
+}
+
.question-wrapper {
position: absolute;
}
diff --git a/src/components/stage/stage.jsx b/src/components/stage/stage.jsx
index ed21d930b..47981e04a 100644
--- a/src/components/stage/stage.jsx
+++ b/src/components/stage/stage.jsx
@@ -21,25 +21,37 @@ const StageComponent = props => {
onQuestionAnswered,
...boxProps
} = props;
- let heightCorrectedAspect = window.innerHeight - 40;
- let widthCorrectedAspect = heightCorrectedAspect + (heightCorrectedAspect / 3);
- if (widthCorrectedAspect > window.innerWidth) {
- widthCorrectedAspect = window.innerWidth;
- heightCorrectedAspect = widthCorrectedAspect * .75;
+
+ let heightCorrectedAspect = height;
+ let widthCorrectedAspect = width;
+ const spacingBorderAdjustment = 9;
+ const stageMenuHeightAdjustment = 40;
+ if (isZoomed) {
+ heightCorrectedAspect = window.innerHeight - stageMenuHeightAdjustment - spacingBorderAdjustment;
+ widthCorrectedAspect = heightCorrectedAspect + (heightCorrectedAspect / 3);
+ if (widthCorrectedAspect > window.innerWidth) {
+ widthCorrectedAspect = window.innerWidth;
+ heightCorrectedAspect = widthCorrectedAspect * .75;
+ }
}
- return isZoomed === false ? (
+ return (
<div>
<Box
- className={classNames(styles.stageWrapper, {
- [styles.withColorPicker]: isColorPicking
+ className={classNames({
+ [styles.stageWrapper]: !isZoomed,
+ [styles.stageWrapperOverlay]: isZoomed,
+ [styles.withColorPicker]: !isZoomed && isColorPicking
})}
>
<Box
- className={styles.stage}
+ className={classNames(
+ styles.stage,
+ {[styles.stageOverlayContent]: isZoomed}
+ )}
componentRef={canvasRef}
element="canvas"
- height={height}
- width={width}
+ height={heightCorrectedAspect}
+ width={widthCorrectedAspect}
{...boxProps}
/>
<Box className={styles.monitorWrapper}>
@@ -51,47 +63,30 @@ const StageComponent = props => {
</Box>
) : null}
{question === null ? null : (
- <Question
- question={question}
- onQuestionAnswered={onQuestionAnswered}
- />
- )}
- </Box>
- {isColorPicking ? (
- <Box
- className={styles.colorPickerBackground}
- onClick={onDeactivateColorPicker}
- />
- ) : null}
- </div>
- ) : (
- <div>
- <Box className={styles.stageWrapperOverlay}>
- <Box
- className={classNames(styles.stage, styles.stageOverlayContent)}
- componentRef={canvasRef}
- element="canvas"
- height={heightCorrectedAspect}
- width={widthCorrectedAspect}
- {...boxProps}
- />
- <Box className={styles.monitorWrapper}>
- <MonitorList />
- </Box>
- <div className={styles.stageOverlayContent}>
<div
- className={styles.questionWrapper}
- style={{width: widthCorrectedAspect}}
+ className={classNames(
+ styles.stageOverlayContent,
+ styles.stageOverlayContentBorderOverride
+ )}
>
- {question === null ? null : (
+ <div
+ className={styles.questionWrapper}
+ style={{width: widthCorrectedAspect}}
+ >
<Question
question={question}
onQuestionAnswered={onQuestionAnswered}
/>
- )}
+ </div>
</div>
- </div>
+ )}
</Box>
+ {isColorPicking ? (
+ <Box
+ className={styles.colorPickerBackground}
+ onClick={onDeactivateColorPicker}
+ />
+ ) : null}
</div>
);
};
diff --git a/src/containers/stage-header.jsx b/src/containers/stage-header.jsx
index 2bea636af..d3acba94e 100644
--- a/src/containers/stage-header.jsx
+++ b/src/containers/stage-header.jsx
@@ -7,12 +7,8 @@ import {connect} from 'react-redux';
import StageHeaderComponent from '../components/stage-header/stage-header.jsx';
+// eslint-disable-next-line react/prefer-stateless-function
class StageHeader extends React.Component {
- shouldComponentUpdate (nextProps) {
- return this.props.width !== nextProps.width ||
- this.props.height !== nextProps.height ||
- this.props.isZoomed !== nextProps.isZoomed;
- }
render () {
const {
...props
--
GitLab