Skip to content

Mobile Sketch Preview Screen #1467

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 30 commits into from
Jul 1, 2020
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
0d119aa
:construction: emplace settings and play icons
ghalestrilo Jun 16, 2020
bdedc63
:construction: add icons to header
ghalestrilo Jun 16, 2020
0877d39
:construction: split components into files
ghalestrilo Jun 16, 2020
5c80702
:construction: add MobileSketchView screen to /mobile/preview
ghalestrilo Jun 18, 2020
49a9fe7
:construction: make link to /mobile
ghalestrilo Jun 18, 2020
e5bbb53
:construction: create alternate content wrapper for proper top padding
ghalestrilo Jun 18, 2020
0633c3b
:construction: mount <PreviewFrame /> on /mobile/preview
ghalestrilo Jun 18, 2020
e11756d
:construction: correct hook call for getProject
ghalestrilo Jun 19, 2020
8a067a8
:construction: make mobile IDE screen start the sketch
ghalestrilo Jun 19, 2020
123c2b0
🔀 merge from origin/develop
ghalestrilo Jun 19, 2020
d00506a
:construction: mound the <PreviewFrame /> properly on Mobile Preview …
ghalestrilo Jun 19, 2020
a11bc44
:sparkles: render sketch
ghalestrilo Jun 19, 2020
776c3d2
:recycle: refactor call to PreviewFrame#renderSketch
ghalestrilo Jun 19, 2020
eb3bc59
:broom: clean up routes.js
ghalestrilo Jun 19, 2020
0e66756
:bug: correct projectName propType to string
ghalestrilo Jun 19, 2020
597cb9b
:sparkles: strech preview canvas to full-width on demand
ghalestrilo Jun 19, 2020
7604e27
:ok_hand: mark comment on routes.jsx as TODO
ghalestrilo Jun 29, 2020
ec8566f
:ok_hand: remove empty useEffect
ghalestrilo Jun 29, 2020
ebb9525
:ok_hand: move icons to common/icons
ghalestrilo Jun 29, 2020
75bd5a3
:ok_hand: remove eslint-disable
ghalestrilo Jun 29, 2020
1eb1bff
:ok_hand: remove debugger comment
ghalestrilo Jun 29, 2020
c39211d
:ok_hand: remove linter comment
ghalestrilo Jun 29, 2020
78ec304
:ok_hand: remove eslint-disable
ghalestrilo Jun 29, 2020
9ca0995
:ok_hand: create Panel color object in theme
ghalestrilo Jun 29, 2020
7d24c07
:ok_hand: use common/Button component for IconButton
ghalestrilo Jun 29, 2020
8a0b09d
:ok_hand: use common/Button component for IconButton on screens
ghalestrilo Jun 29, 2020
7805acc
:bug: fix margin on header title
ghalestrilo Jun 29, 2020
a1d6abf
:ok_hand: rename Panel to MobilePanel on theme.js
ghalestrilo Jul 1, 2020
3143cc3
:ok_hand: change IconButton structure
ghalestrilo Jul 1, 2020
b4c1b86
:ok_hand: improve IconButton implementation
ghalestrilo Jul 1, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion client/common/icons.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import React from 'react';
import PropTypes from 'prop-types';
import styled from 'styled-components';
import { remSize, prop } from '../theme';
import { prop } from '../theme';
import SortArrowUp from '../images/sort-arrow-up.svg';
import SortArrowDown from '../images/sort-arrow-down.svg';
import Github from '../images/github.svg';
Expand All @@ -10,6 +10,8 @@ import Plus from '../images/plus-icon.svg';
import Close from '../images/close.svg';
import Exit from '../images/exit.svg';
import DropdownArrow from '../images/down-filled-triangle.svg';
import Preferences from '../images/preferences.svg';
import Play from '../images/triangle-arrow-right.svg';

// HOC that adds the right web accessibility props
// https://www.scottohara.me/blog/2019/05/22/contextual-images-svgs-and-a11y.html
Expand Down Expand Up @@ -70,3 +72,5 @@ export const PlusIcon = withLabel(Plus);
export const CloseIcon = withLabel(Close);
export const ExitIcon = withLabel(Exit);
export const DropdownArrowIcon = withLabel(DropdownArrow);
export const PreferencesIcon = withLabel(Preferences);
export const PlayIcon = withLabel(Play);
20 changes: 20 additions & 0 deletions client/components/mobile/Footer.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
import React from 'react';
import styled from 'styled-components';
import { prop, remSize } from '../../theme';

const background = prop('Panel.default.background');
const textColor = prop('primaryTextColor');

const Footer = styled.div`
position: fixed;
width: 100%;
background: ${background};
color: ${textColor};
padding: ${remSize(12)};
padding-left: ${remSize(32)};
z-index: 1;

bottom: 0;
`;

export default Footer;
30 changes: 30 additions & 0 deletions client/components/mobile/Header.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import React from 'react';
import styled from 'styled-components';
import { prop, remSize } from '../../theme';

const background = prop('Panel.default.background');
const textColor = prop('primaryTextColor');

const Header = styled.div`
position: fixed;
width: 100%;
background: ${background};
color: ${textColor};
padding: ${remSize(12)};
padding-left: ${remSize(16)};
padding-right: ${remSize(16)};
z-index: 1;

display: flex;
flex: 1;
flex-direction: row;
justify-content: flex-start;
align-items: center;

// TODO:
svg {
height: 2rem;
}
`;

export default Header;
8 changes: 8 additions & 0 deletions client/components/mobile/IDEWrapper.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import React from 'react';
import styled from 'styled-components';
import { remSize } from '../../theme';

export default styled.div`
z-index: 0;
margin-top: ${remSize(16)};
`;
24 changes: 24 additions & 0 deletions client/components/mobile/IconButton.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import React from 'react';
import PropTypes from 'prop-types';
import styled from 'styled-components';
import Button from '../../common/Button';

const ButtonWrapper = styled(Button)`
width: 3rem;
> svg {
width: 100%;
height: 100%;
}
`;

const IconButton = props => (<ButtonWrapper
iconBefore={props.children}
kind={Button.kinds.inline}
{...{ ...props, children: null }}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you need to put children: null here? Isn't children null here if you just don't put anything in between <ButtonWrapper></ButtonWrapper>?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's null if I don't pass anything, but in this case I wanted to pass children into the iconBefore prop, and not into the body of the component.
I thought of a better solution, so I'll change this soon

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I introduced an icon prop, now it looks a lot cleaner

/>);

IconButton.propTypes = {
children: PropTypes.element.isRequired
};

export default IconButton;
13 changes: 13 additions & 0 deletions client/components/mobile/MobileScreen.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import React from 'react';
import PropTypes from 'prop-types';

const Screen = ({ children }) => (
<div className="fullscreen-preview">
{children}
</div>
);
Screen.propTypes = {
children: PropTypes.node.isRequired
};

export default Screen;
72 changes: 34 additions & 38 deletions client/modules/IDE/components/PreviewFrame.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,23 @@ import {
import { hijackConsoleErrorsScript, startTag, getAllScriptOffsets }
from '../../../utils/consoleUtils';


const shouldRenderSketch = (props, prevProps = undefined) => {
const { isPlaying, previewIsRefreshing, fullView } = props;

// if the user explicitly clicks on the play button
if (isPlaying && previewIsRefreshing) return true;

if (!prevProps) return false;

return (props.isPlaying !== prevProps.isPlaying // if sketch starts or stops playing, want to rerender
|| props.isAccessibleOutputPlaying !== prevProps.isAccessibleOutputPlaying // if user switches textoutput preferences
|| props.textOutput !== prevProps.textOutput
|| props.gridOutput !== prevProps.gridOutput
|| props.soundOutput !== prevProps.soundOutput
|| (fullView && props.files[0].id !== prevProps.files[0].id));
};

class PreviewFrame extends React.Component {
constructor(props) {
super(props);
Expand All @@ -30,46 +47,17 @@ class PreviewFrame extends React.Component {

componentDidMount() {
window.addEventListener('message', this.handleConsoleEvent);

const props = {
...this.props,
previewIsRefreshing: this.props.previewIsRefreshing,
isAccessibleOutputPlaying: this.props.isAccessibleOutputPlaying
};
if (shouldRenderSketch(props)) this.renderSketch();
}

componentDidUpdate(prevProps) {
// if sketch starts or stops playing, want to rerender
if (this.props.isPlaying !== prevProps.isPlaying) {
this.renderSketch();
return;
}

// if the user explicitly clicks on the play button
if (this.props.isPlaying && this.props.previewIsRefreshing) {
this.renderSketch();
return;
}

// if user switches textoutput preferences
if (this.props.isAccessibleOutputPlaying !== prevProps.isAccessibleOutputPlaying) {
this.renderSketch();
return;
}

if (this.props.textOutput !== prevProps.textOutput) {
this.renderSketch();
return;
}

if (this.props.gridOutput !== prevProps.gridOutput) {
this.renderSketch();
return;
}

if (this.props.soundOutput !== prevProps.soundOutput) {
this.renderSketch();
return;
}

if (this.props.fullView && this.props.files[0].id !== prevProps.files[0].id) {
this.renderSketch();
}

if (shouldRenderSketch(this.props, prevProps)) this.renderSketch();
// small bug - if autorefresh is on, and the usr changes files
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a bug that still exists? Should this comment be removed?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the commend holds, since I merely moved code around. The logic is pretty much the same

// in the sketch, preview will reload
}
Expand Down Expand Up @@ -212,6 +200,12 @@ class PreviewFrame extends React.Component {
this.addLoopProtect(sketchDoc);
sketchDoc.head.insertBefore(consoleErrorsScript, sketchDoc.head.firstElement);

if (this.props.forceFullWidth) {
const resizeScript = sketchDoc.createElement('style');
resizeScript.innerHTML = '.p5Canvas { width: 100% !important; height: auto !important }';
sketchDoc.head.appendChild(resizeScript);
}

return `<!DOCTYPE HTML>\n${sketchDoc.documentElement.outerHTML}`;
}

Expand Down Expand Up @@ -395,11 +389,13 @@ PreviewFrame.propTypes = {
clearConsole: PropTypes.func.isRequired,
cmController: PropTypes.shape({
getContent: PropTypes.func
})
}),
forceFullWidth: PropTypes.bool
};

PreviewFrame.defaultProps = {
fullView: false,
forceFullWidth: false,
cmController: {}
};

Expand Down
96 changes: 33 additions & 63 deletions client/modules/IDE/pages/IDEViewMobile.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,93 +20,61 @@ import { getHTMLFile } from '../reducers/files';

// Local Imports
import Editor from '../components/Editor';
import { prop, remSize } from '../../../theme';
import { ExitIcon } from '../../../common/icons';

const background = prop('Button.default.background');
const textColor = prop('primaryTextColor');
import { PreferencesIcon, PlayIcon } from '../../../common/icons';

import IconButton from '../../../components/mobile/IconButton';
import Header from '../../../components/mobile/Header';
import Screen from '../../../components/mobile/MobileScreen';
import Footer from '../../../components/mobile/Footer';
import IDEWrapper from '../../../components/mobile/IDEWrapper';

const Header = styled.div`
position: fixed;
width: 100%;
background: ${background};
color: ${textColor};
padding: ${remSize(12)};
padding-left: ${remSize(32)};
padding-right: ${remSize(32)};
z-index: 1;

const IconContainer = styled.div`
marginLeft: 2rem;
display: flex;
flex: 1;
flex-direction: row;
justify-content: flex-start;
align-items: center;
`;

const Footer = styled.div`
position: fixed;
width: 100%;
background: ${background};
color: ${textColor};
padding: ${remSize(12)};
padding-left: ${remSize(32)};
z-index: 1;
const TitleContainer = styled.div`

bottom: 0;
`;

const Content = styled.div`
z-index: 0;
margin-top: ${remSize(16)};
`;

const Icon = styled.a`
> svg {
fill: ${textColor};
color: ${textColor};
margin-left: ${remSize(16)};
}
`;

const IconLinkWrapper = styled(Link)`
width: 3rem;
margin-right: 1.25rem;
margin-left: none;
`;


const Screen = ({ children }) => (
<div className="fullscreen-preview">
{children}
</div>
);
Screen.propTypes = {
children: PropTypes.node.isRequired
};

const isUserOwner = ({ project, user }) => (project.owner && project.owner.id === user.id);

const IDEViewMobile = (props) => {
const {
preferences, ide, editorAccessibility, project, updateLintMessage, clearLintMessage, selectedFile, updateFileContent, files, closeEditorOptions, showEditorOptions, showKeyboardShortcutModal, setUnsavedChanges, startRefreshSketch, stopSketch, expandSidebar, collapseSidebar, clearConsole, console, showRuntimeErrorWarning, hideRuntimeErrorWarning
preferences, ide, editorAccessibility, project, updateLintMessage, clearLintMessage,
selectedFile, updateFileContent, files,
closeEditorOptions, showEditorOptions, showKeyboardShortcutModal, setUnsavedChanges,
startRefreshSketch, stopSketch, expandSidebar, collapseSidebar, clearConsole, console,
showRuntimeErrorWarning, hideRuntimeErrorWarning, startSketch
} = props;

const [tmController, setTmController] = useState(null);
const [tmController, setTmController] = useState(null); // eslint-disable-line
const [overlay, setOverlay] = useState(null); // eslint-disable-line

return (
<Screen>
<Header>
<IconLinkWrapper to="/" aria-label="Return to original editor">
<ExitIcon />
</IconLinkWrapper>
<div>
<IconButton to="/mobile" aria-label="Return to original editor">
<ExitIcon viewBox="0 0 16 16" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are you using viewbox here to change the size of the icon? I would prefer if you explicitly set the width and height, and maybe this needs to be added to client/common/icons.jsx, which could be done in a separate PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll remove the viewbox prop for now, and find a way to resize the icon in a future PR, to minimize conflicts with the other branch

</IconButton>
<div style={{ marginLeft: '1rem' }}>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if possible i would prefer to not have inline styles!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The next PR fixes this by improving the structure of the <Header /> component

<h2>{project.name}</h2>
<h3>{selectedFile.name}</h3>
</div>

<IconContainer>
<IconButton onClick={() => setOverlay('preferences')}>
<PreferencesIcon focusable="false" aria-hidden="true" />
</IconButton>
<IconButton to="/mobile/preview" onClick={() => { startSketch(); }}>
<PlayIcon viewBox="-1 -1 7 7" focusable="false" aria-hidden="true" />
</IconButton>
</IconContainer>
</Header>

<Content>
<IDEWrapper>
<Editor
lintWarning={preferences.lintWarning}
linewrap={preferences.linewrap}
Expand Down Expand Up @@ -141,7 +109,7 @@ const IDEViewMobile = (props) => {
runtimeErrorWarningVisible={ide.runtimeErrorWarningVisible}
provideController={setTmController}
/>
</Content>
</IDEWrapper>
<Footer><h2>Bottom Bar</h2></Footer>
</Screen>
);
Expand Down Expand Up @@ -205,6 +173,8 @@ IDEViewMobile.propTypes = {
updatedAt: PropTypes.string
}).isRequired,

startSketch: PropTypes.func.isRequired,

updateLintMessage: PropTypes.func.isRequired,

clearLintMessage: PropTypes.func.isRequired,
Expand Down
Loading