-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
feat($urlParams): add params for isInProjectView + initialPath #49
Changes from 11 commits
fb181c1
1704c37
7ca8118
9b8f13e
5c64bfa
ba319dc
ba900a2
ef70681
d571d52
0a11495
89ce1ce
889b5b9
27e0ae1
f07d0d0
9a64f62
f9bc699
1d6fb21
9089f68
cd0d70b
6d28e42
8499746
7e0526b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -92,4 +92,4 @@ export default ({ | |
</Tooltip> | ||
</SwitchContainer>} | ||
</Container> | ||
); | ||
); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,7 @@ const StyledFrame = styled.iframe` | |
|
||
type Props = { | ||
sandboxId: string, | ||
initialPath: ?string, | ||
isInProjectView: boolean, | ||
modules: Array<Module>, | ||
directories: Array<Directory>, | ||
|
@@ -56,20 +57,27 @@ type State = { | |
}; | ||
|
||
export default class Preview extends React.PureComponent { | ||
initialPath: string | ||
|
||
constructor(props: Props) { | ||
super(props); | ||
|
||
this.state = { | ||
frameInitialized: false, | ||
history: [], | ||
historyPosition: 0, | ||
urlInAddressBar: '', | ||
urlInAddressBar: props.initialPath || '', | ||
url: null, | ||
}; | ||
|
||
if (!props.noDelay) { | ||
this.executeCode = debounce(this.executeCode, 800); | ||
} | ||
|
||
// we need a value that doesn't change when receiving `initialPath` | ||
// from the query params, or the iframe will continue to be re-rendered | ||
// when the user navigates the iframe app, which shows the loading screen | ||
this.initialPath = this.state.urlInAddressBar | ||
} | ||
|
||
static defaultProps = { | ||
|
@@ -158,7 +166,8 @@ export default class Preview extends React.PureComponent { | |
const element = document.getElementById('sandbox'); | ||
|
||
if (element) { | ||
element.contentWindow.postMessage(message, frameUrl()); | ||
const { urlInAddressBar: url } = this.state | ||
element.contentWindow.postMessage(message, frameUrl(url)); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think this needs to have the url, it's a check for targetOrigin (https://developer.mozilla.org/en-US/docs/Web/API/Window/postMessage). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes I think so too. Undo it |
||
} | ||
}; | ||
|
||
|
@@ -252,6 +261,7 @@ export default class Preview extends React.PureComponent { | |
const { history, historyPosition } = this.state; | ||
|
||
document.getElementById('sandbox').src = frameUrl(history[historyPosition]); | ||
|
||
this.setState({ | ||
urlInAddressBar: history[historyPosition], | ||
}); | ||
|
@@ -350,7 +360,7 @@ export default class Preview extends React.PureComponent { | |
/>} | ||
<StyledFrame | ||
sandbox="allow-forms allow-scripts allow-same-origin allow-modals allow-popups allow-presentation" | ||
src={frameUrl()} | ||
src={frameUrl(this.initialPath)} | ||
id="sandbox" | ||
hideNavigation={hideNavigation} | ||
/> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -29,6 +29,14 @@ const Container = styled.div` | |
height: 100%; | ||
`; | ||
|
||
const CancelDefaultModule = styled.div` | ||
text-decoration: underline; | ||
color: rgb(82, 174, 217); | ||
position: relative; | ||
top: -10px; | ||
cursor: pointer; | ||
` | ||
|
||
const FilesContainer = styled.div` | ||
max-height: 300px; | ||
overflow: auto; | ||
|
@@ -144,15 +152,31 @@ const mapStateToProps = createSelector( | |
); | ||
class ShareView extends React.PureComponent { | ||
props: Props; | ||
state = { | ||
showEditor: true, | ||
showPreview: true, | ||
defaultModule: null, | ||
autoResize: false, | ||
hideNavigation: false, | ||
fontSize: 14, | ||
}; | ||
|
||
constructor(props, context) { | ||
super(props, context); | ||
|
||
this.state = { | ||
showEditor: true, | ||
showPreview: true, | ||
defaultModule: null, | ||
autoResize: false, | ||
hideNavigation: false, | ||
isFileEval: !props.sandbox.isInProjectView, // thats why i moved this to the constructor :) | ||
fontSize: 14, | ||
initialPath: '' | ||
} | ||
} | ||
|
||
componentWillReceiveProps(nextProps) { | ||
const { sandbox: { isInProjectView } } = nextProps | ||
|
||
if (isInProjectView !== this.props.sandbox.isInProjectView) { | ||
this.setState({ isFileEval: !isInProjectView }) | ||
} | ||
} | ||
|
||
|
||
handleChange = e => this.setState({ message: e.target.value }); | ||
|
||
handleSend = () => { | ||
|
@@ -169,6 +193,7 @@ class ShareView extends React.PureComponent { | |
setMixedView = () => this.setState({ showEditor: true, showPreview: true }); | ||
|
||
setDefaultModule = id => this.setState({ defaultModule: id }); | ||
clearDefaultModule = () => this.setState({ defaultModule: null }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think that it's more clear for the user if we select the main module by default, and don't append There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree. Cleaner URLs are better. Observant developers will pay attention to the query params. But I didn't know how to do it with proper guarantees. I couldn't be sure that with no id selected it worked since I'm new to the codebase. I wasn't sure what else u were doing. Anyway u made this decision early on, it's probably easier for me now. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you could add a check to the url generator like |
||
|
||
getOptionsUrl = () => { | ||
const { | ||
|
@@ -177,7 +202,9 @@ class ShareView extends React.PureComponent { | |
showPreview, | ||
autoResize, | ||
hideNavigation, | ||
isFileEval, | ||
fontSize, | ||
initialPath | ||
} = this.state; | ||
|
||
const options = {}; | ||
|
@@ -201,10 +228,18 @@ class ShareView extends React.PureComponent { | |
options.hidenavigation = 1; | ||
} | ||
|
||
if (isFileEval) { | ||
options.isFileEval = 1; | ||
} | ||
|
||
if (fontSize !== 14) { | ||
options.fontsize = fontSize; | ||
} | ||
|
||
if (initialPath) { | ||
options.initialPath = initialPath; | ||
} | ||
|
||
return optionsToParameterizedUrl(options); | ||
}; | ||
|
||
|
@@ -219,6 +254,11 @@ class ShareView extends React.PureComponent { | |
|
||
return protocolAndHost() + embedUrl(sandbox) + this.getOptionsUrl(); | ||
}; | ||
|
||
setInitialPath = ({ target }) => { | ||
const initialPath = target.value | ||
this.setState({ initialPath }) | ||
}; | ||
|
||
getIframeScript = () => | ||
`<iframe src="${this.getEmbedUrl()}" style="width:100%; height:500px; border:0; border-radius: 4px; overflow:hidden;" sandbox="allow-modals allow-forms allow-popups allow-scripts allow-same-origin"></iframe>`; | ||
|
@@ -251,16 +291,23 @@ class ShareView extends React.PureComponent { | |
this.setState({ hideNavigation }); | ||
}; | ||
|
||
setIsFileEval = (isFileEval: boolean) => { | ||
this.setState({ isFileEval }); | ||
}; | ||
|
||
setFontSize = (fontSize: number) => [this.setState({ fontSize })]; | ||
|
||
render() { | ||
const { sandbox, modules, directories } = this.props; | ||
|
||
const { | ||
showEditor, | ||
showPreview, | ||
autoResize, | ||
hideNavigation, | ||
isFileEval, | ||
fontSize, | ||
initialPath | ||
} = this.state; | ||
|
||
const defaultModule = | ||
|
@@ -295,12 +342,26 @@ class ShareView extends React.PureComponent { | |
value={hideNavigation} | ||
setValue={this.setHideNavigation} | ||
/> | ||
<PaddedPreference | ||
title="File eval" | ||
value={isFileEval} | ||
setValue={this.setIsFileEval} | ||
/> | ||
<PaddedPreference | ||
title="Font size" | ||
value={fontSize} | ||
setValue={this.setFontSize} | ||
/> | ||
</div> | ||
<Inputs> | ||
<LinkName>Project Initial Path</LinkName> | ||
<input | ||
onFocus={this.select} | ||
placeholder='e.g: /home' | ||
value={initialPath} | ||
onChange={this.setInitialPath} | ||
/> | ||
</Inputs> | ||
<div> | ||
<h4>Default view</h4> | ||
<div | ||
|
@@ -321,7 +382,12 @@ class ShareView extends React.PureComponent { | |
</div> | ||
</div> | ||
<div> | ||
<h4>Default module to show and preview</h4> | ||
<h4>Default module to show</h4> | ||
{this.state.defaultModule && | ||
<CancelDefaultModule onClick={this.clearDefaultModule}> | ||
cancel | ||
</CancelDefaultModule> | ||
} | ||
|
||
<FilesContainer> | ||
<Files | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -72,7 +72,7 @@ export default class DirectoryChildren extends React.PureComponent { | |
{modules.filter(x => x.directoryShortid === parentShortid).map(m => { | ||
const isActive = m.id === currentModuleId; | ||
const mainModule = isMainModule(m); | ||
|
||
console.log('is ACTIVE', isActive, isActive && m) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh one last thing, could you remove this line? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. done |
||
const hasError = !!errors.find( | ||
e => e.severity === 'error' && e.moduleId === m.id, | ||
); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ import { DragDropContext } from 'react-dnd'; | |
import { | ||
modulesFromSandboxSelector, | ||
findMainModule, | ||
findCurrentModule, | ||
} from 'app/store/entities/sandboxes/modules/selectors'; | ||
import { directoriesFromSandboxSelector } from 'app/store/entities/sandboxes/directories/selectors'; | ||
|
||
|
@@ -47,12 +48,11 @@ class Files extends React.PureComponent { | |
|
||
render() { | ||
const { sandbox, modules, directories } = this.props; | ||
if (sandbox == null) return null; | ||
|
||
const mainModule = findMainModule(modules); | ||
|
||
const { currentModule } = sandbox; | ||
|
||
if (sandbox == null) return null; | ||
const { currentModule: currentModuleId } = sandbox; | ||
const currentModule = findCurrentModule(modules, currentModuleId, mainModule); | ||
|
||
return ( | ||
<DirectoryEntry | ||
|
@@ -62,8 +62,8 @@ class Files extends React.PureComponent { | |
modules={sortBy(modules, 'title')} | ||
directories={sortBy(directories, 'title')} | ||
isInProjectView={sandbox.isInProjectView} | ||
// $FlowIssue | ||
currentModuleId={currentModule || mainModule.id} | ||
currentModuleId={currentModule.id} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is nice, much cleaner. |
||
currentDirectoryShortid={currentModule.directoryShortid} | ||
errors={sandbox.errors} | ||
id={null} | ||
shortid={null} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you mean with this navigating inside the iframe, or manually entering the url?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I meant navigating when the app uses history.push like my sandbox. It was refreshing because the URL kept changing. It kept changing because no longer was passed just the base URL.
So I fixed it by making sure it got the URL, but only once on mount. That way it can receive initialPath from query params. It doesn't need to think about it anymore after the first render. Put back urlInAddressBar and try clicking links in my codesandbox to see.