From d43658f1d948c0dfa6e64767a35182b8f9d8e63c Mon Sep 17 00:00:00 2001
From: Karishma Chadha <kchadha@scratch.mit.edu>
Date: Thu, 22 Feb 2018 11:51:30 -0500
Subject: [PATCH] Code cleanup and adding tests. Adding todos for next steps.
---
src/components/import-error/import-error.jsx | 21 ++++++++++--
src/components/import-input/import-input.jsx | 3 +-
src/containers/import-modal.jsx | 22 +++---------
test/integration/project-loading.test.js | 36 +++++++++++++++++++-
4 files changed, 60 insertions(+), 22 deletions(-)
diff --git a/src/components/import-error/import-error.jsx b/src/components/import-error/import-error.jsx
index bc8f287ea..af677e4a2 100644
--- a/src/components/import-error/import-error.jsx
+++ b/src/components/import-error/import-error.jsx
@@ -1,6 +1,6 @@
import bindAll from 'lodash.bindall';
import classNames from 'classnames';
-import {/* defineMessages, */injectIntl/* , intlShape, FormattedMessage*/} from 'react-intl';
+import {defineMessages, injectIntl, FormattedMessage} from 'react-intl';
import PropTypes from 'prop-types';
import React from 'react';
import ReactTooltip from 'react-tooltip';
@@ -10,6 +10,17 @@ import styles from './import-error.css';
// TODO store different error messages depending on the situation (?) and
// needs to use intl lib for localization support
+// TODO error tooltip should be always visible in the error state,
+// instead of popping up hen hovering over the input
+
+const messages = defineMessages({
+ invalidLink: {
+ id: 'gui.importError.invalidLink',
+ defaultMessage: 'Uh oh, that link doesn\'t look quite right.',
+ description: 'Invalid link error message'
+ }
+});
+
class ImportErrorContent extends React.Component {
constructor (props) {
super(props);
@@ -30,8 +41,13 @@ class ImportErrorContent extends React.Component {
this.setState({isShowing: false});
}
getContent () {
+ const messageId = this.props.errorMessage;
return (
- <p>{this.props.errorMessage}</p>
+ <p>
+ <FormattedMessage
+ {...messages[`${messageId}`]}
+ />
+ </p>
);
}
render () {
@@ -60,7 +76,6 @@ class ImportErrorContent extends React.Component {
ImportErrorContent.propTypes = {
className: PropTypes.string,
errorMessage: PropTypes.string.isRequired,
- // intl: intlShape,
place: PropTypes.oneOf(['top', 'right', 'bottom', 'left']),
tooltipId: PropTypes.string.isRequired
diff --git a/src/components/import-input/import-input.jsx b/src/components/import-input/import-input.jsx
index bc4d7c0c2..21ea902a0 100644
--- a/src/components/import-input/import-input.jsx
+++ b/src/components/import-input/import-input.jsx
@@ -1,10 +1,11 @@
import PropTypes from 'prop-types';
import React from 'react';
-// import classNames from 'classnames';
import {ImportErrorTooltip} from '../import-error/import-error.jsx';
+// TODO error tooltip needs to go around both the input and the button
+// error tooltip should also be always visible
class ImportInput extends React.Component {
render () {
let input = null;
diff --git a/src/containers/import-modal.jsx b/src/containers/import-modal.jsx
index e48df8e5a..169378e1a 100644
--- a/src/containers/import-modal.jsx
+++ b/src/containers/import-modal.jsx
@@ -2,12 +2,8 @@ import bindAll from 'lodash.bindall';
import PropTypes from 'prop-types';
import React from 'react';
import {connect} from 'react-redux';
-import platform from 'platform';
import ImportModalComponent from '../components/import-modal/import-modal.jsx';
-import BrowserModalComponent from '../components/browser-modal/browser-modal.jsx';
-
-import log from '../lib/log';
import {
closeImportInfo
@@ -44,10 +40,10 @@ class ImportModal extends React.Component {
document.body.removeChild(projectLink);
this.props.onViewProject();
} else {
+ // TODO handle error messages and error states
this.setState({
hasValidationError: true,
- errorMessage: `Uh oh, that link doesn't look quite right.`});
- log.info('Invalid link error');
+ errorMessage: `invalidLink`});
}
}
handleChange (e) {
@@ -56,7 +52,6 @@ class ImportModal extends React.Component {
validate (input) {
const matches = input.match(/^(https:\/\/)?scratch\.mit\.edu\/projects\/(\d+)(\/?)$/);
if (matches && matches.length > 0) {
- log.info(`Project ID: ${matches[2]}`);
return matches[2];
}
return null;
@@ -65,18 +60,14 @@ class ImportModal extends React.Component {
this.props.onCancel();
}
handleGoBack () {
+ // TODO what should the go back button actually do? Should it bring the preview modal
+ // back up or just close this modal, or go back to scratch?
window.location.replace('https://scratch.mit.edu');
}
// TODO not sure if we need this, since it shouldn't be possible to bring up this
// modal without first going through the preview modal
- supportedBrowser () {
- if (platform.name === 'IE') {
- return false;
- }
- return true;
- }
render () {
- return (this.supportedBrowser() ?
+ return (
<ImportModalComponent
errorMessage={this.state.errorMessage}
hasValidationError={this.state.hasValidationError}
@@ -86,9 +77,6 @@ class ImportModal extends React.Component {
onChange={this.handleChange}
onKeyPress={this.handleKeyPress}
onViewProject={this.handleViewProject}
- /> :
- <BrowserModalComponent
- onBack={this.handleGoBack}
/>
);
}
diff --git a/test/integration/project-loading.test.js b/test/integration/project-loading.test.js
index 9d5cb0384..6add67c42 100644
--- a/test/integration/project-loading.test.js
+++ b/test/integration/project-loading.test.js
@@ -4,6 +4,7 @@ import SeleniumHelper from '../helpers/selenium-helper';
const {
clickText,
clickXpath,
+ findByXpath,
getDriver,
getLogs,
loadUri
@@ -31,7 +32,40 @@ describe('Loading scratch gui', () => {
describe('Loading projects by ID', () => {
- test('Load a project by ID', async () => {
+ test('Load 2.0 project using import modal', async () => {
+ await loadUri(uri);
+ await clickText('View 2.0 Project');
+ const el = await findByXpath("//input[@placeholder='scratch.mit.edu/projects/123456789']");
+ const projectId = '96708228';
+ await el.sendKeys(`scratch.mit.edu/projects/${projectId}`);
+ await clickXpath('//button[@title="viewproject"]');
+ await new Promise(resolve => setTimeout(resolve, 2000));
+ await clickXpath('//img[@title="Go"]');
+ await new Promise(resolve => setTimeout(resolve, 2000));
+ await clickXpath('//img[@title="Stop"]');
+ const logs = await getLogs();
+ await expect(logs).toEqual([]);
+ });
+
+ test('Invalid url when loading project through modal lets you try again', async () => {
+ await loadUri(uri);
+ await clickText('View 2.0 Project');
+ let el = await findByXpath("//input[@placeholder='scratch.mit.edu/projects/123456789']");
+ await el.sendKeys('thisisnotaurl');
+ await clickXpath('//button[@title="viewproject"]');
+ el = await findByXpath("//input[@placeholder='scratch.mit.edu/projects/123456789']");
+ await el.clear();
+ await el.sendKeys('scratch.mit.edu/projects/96708228');
+ await clickXpath('//button[@title="viewproject"]');
+ await new Promise(resolve => setTimeout(resolve, 2000));
+ await clickXpath('//img[@title="Go"]');
+ await new Promise(resolve => setTimeout(resolve, 2000));
+ await clickXpath('//img[@title="Stop"]');
+ const logs = await getLogs();
+ await expect(logs).toEqual([]);
+ });
+
+ test('Load a project by ID directly through url', async () => {
const projectId = '96708228';
await loadUri(`${uri}#${projectId}`);
await clickXpath('//button[@title="tryit"]');
--
GitLab