Skip to content

Add restart UI #568

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.

Sign up for GitHub

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 1 commit into from
Jun 9, 2016
Merged

Add restart UI #568

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
22 changes: 21 additions & 1 deletion src/disco/components/Addon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ import {
THEME_PREVIEW,
THEME_RESET_PREVIEW,
THEME_TYPE,
UNINSTALLING,
UNINSTALLED,
UNINSTALL_CATEGORY,
} from 'disco/constants';
Expand Down Expand Up @@ -56,6 +57,7 @@ export class Addon extends React.Component {
iconUrl: PropTypes.string,
id: PropTypes.string.isRequired,
installURL: PropTypes.string,
needsRestart: PropTypes.bool.isRequired,
previewURL: PropTypes.string,
name: PropTypes.string.isRequired,
setCurrentStatus: PropTypes.func.isRequired,
Expand All @@ -68,6 +70,7 @@ export class Addon extends React.Component {
static defaultProps = {
// Defaults themeAction to the imported func.
themeAction,
needsRestart: false,
}

componentDidMount() {
Expand All @@ -84,12 +87,18 @@ export class Addon extends React.Component {
}

getError() {
return this.props.status === ERROR ? (<div className="error">
return this.props.status === ERROR ? (<div className="notification error">
<p className="message">{this.errorMessage()}</p>
<a className="close" href="#" onClick={this.closeError}>Close</a>
</div>) : null;
}

getRestart() {
return this.props.needsRestart ? (<div className="notification restart">
<p className="message">{this.restartMessage()}</p>
</div>) : null;
}

getLogo() {
const { iconUrl } = this.props;
if (this.props.type === EXTENSION_TYPE) {
Expand Down Expand Up @@ -140,6 +149,16 @@ export class Addon extends React.Component {
}
}

restartMessage() {
const { status, i18n } = this.props;
switch (status) {
case UNINSTALLING:
return i18n.gettext('This add-on will be uninstalled after you restart Firefox.');
default:
return i18n.gettext('Please restart Firefox to use this add-on.');
}
}

closeError = (e) => {
e.preventDefault();
this.setCurrentStatus();
Expand Down Expand Up @@ -174,6 +193,7 @@ export class Addon extends React.Component {
{this.getLogo()}
<div className="content">
{this.getError()}
{this.getRestart()}
<div className="copy">
<h2
ref="heading"
Expand Down
64 changes: 36 additions & 28 deletions src/disco/css/Addon.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,34 +80,6 @@ $addon-padding: 20px;
flex-grow: 1;
position: relative;

> .error {
align-content: stretch;
align-items: center;
background: #c33c32 url('../img/warning.svg') no-repeat calc(100% - 40px) 50%;
bottom: 0;
color: #fff;
display: flex;
flex-direction: row;
left: 0;
padding: 15px 20px;
position: absolute;
right: 0;
top: 0;
z-index: 10;

.message {
flex-grow: 1;
}

.close {
bottom: 10px;
color: #fff;
display: block;
left: 15px;
padding: 5px;
position: absolute;
}
}

.copy {
padding: 30px 20px;
Expand All @@ -120,4 +92,40 @@ $addon-padding: 20px;
}
}

.notification {
align-content: stretch;
align-items: center;
bottom: 0;
color: #fff;
display: flex;
flex-direction: row;
left: 0;
padding: 15px 20px;
position: absolute;
right: 0;
top: 0;

&.error {
background: #c33c32 url('../img/warning.svg') no-repeat calc(100% - 40px) 50%;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will webpack inline these or do they need to be required?

Copy link
Contributor Author

@muffinresearch muffinresearch Jun 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah it should inline it automagically since it's tiny.

z-index: 20;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why is the error higher than needs restart? So you see an error if it is in ERROR and needsRestart?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, it's very unlikely to be the case that both would be displayed at the same time, but I figured that an error should trump the restart notification.

}

&.restart {
background: #70CF5B url('../img/restart.svg') no-repeat calc(100% - 40px) 50%;
z-index: 10;
}

.message {
flex-grow: 1;
}

.close {
bottom: 10px;
color: #fff;
display: block;
left: 15px;
padding: 5px;
position: absolute;
}
}
}
3 changes: 3 additions & 0 deletions src/disco/img/restart.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
1 change: 1 addition & 0 deletions src/disco/reducers/installations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ export default function installations(state = {}, { type, payload }) {
url: payload.url,
downloadProgress: 0,
status: normalizeStatus(payload.status),
needsRestart: payload.needsRestart || false,
};
} else if (type === START_DOWNLOAD) {
addon.status = DOWNLOADING;
Expand Down
33 changes: 29 additions & 4 deletions tests/client/disco/components/TestAddon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import {
THEME_RESET_PREVIEW,
THEME_TYPE,
UNINSTALLED,
UNINSTALLING,
UNINSTALL_CATEGORY,
} from 'disco/constants';
import { getFakeAddonManagerWrapper, getFakeI18nInst } from 'tests/client/helpers';
Expand Down Expand Up @@ -63,7 +64,7 @@ describe('<Addon />', () => {
it('renders a default error overlay', () => {
const data = {...result, status: ERROR, setCurrentStatus: sinon.stub()};
root = renderAddon(data);
const error = findDOMNode(root).querySelector('.error');
const error = findDOMNode(root).querySelector('.notification.error');
assert.equal(
error.querySelector('p').textContent,
'An unexpected error occurred.',
Expand All @@ -77,7 +78,7 @@ describe('<Addon />', () => {
...result, status: ERROR, error: INSTALL_FAILED, setCurrentStatus: sinon.stub(),
};
root = renderAddon(data);
const error = findDOMNode(root).querySelector('.error');
const error = findDOMNode(root).querySelector('.notification.error');
assert.equal(
error.querySelector('p').textContent,
'Installation failed. Please try again.',
Expand All @@ -91,7 +92,7 @@ describe('<Addon />', () => {
...result, status: ERROR, error: DOWNLOAD_FAILED, setCurrentStatus: sinon.stub(),
};
root = renderAddon(data);
const error = findDOMNode(root).querySelector('.error');
const error = findDOMNode(root).querySelector('.notification.error');
assert.equal(
error.querySelector('p').textContent,
'Download failed. Please check your connection.',
Expand All @@ -101,7 +102,31 @@ describe('<Addon />', () => {
});

it('does not normally render an error', () => {
assert.notOk(findDOMNode(root).querySelector('.error'));
assert.notOk(findDOMNode(root).querySelector('.notification.error'));
});

it('renders a default restart notification', () => {
const data = { ...result, needsRestart: true };
root = renderAddon(data);
const restart = findDOMNode(root).querySelector('.notification.restart');
assert.equal(
restart.querySelector('p').textContent,
'Please restart Firefox to use this add-on.',
'restart message should be present');
});

it('renders a uninstallation restart notification', () => {
const data = { ...result, needsRestart: true, status: UNINSTALLING };
root = renderAddon(data);
const restart = findDOMNode(root).querySelector('.notification.restart');
assert.equal(
restart.querySelector('p').textContent,
'This add-on will be uninstalled after you restart Firefox.',
'restart uninstallation message should be present');
});

it('does not normally render a restart notification', () => {
assert.notOk(findDOMNode(root).querySelector('.notification.restart'));
});

it('renders the heading', () => {
Expand Down
26 changes: 26 additions & 0 deletions tests/client/disco/reducers/test_installations.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,29 @@ describe('installations reducer', () => {
url: 'https://cdn.amo/download/my-addon.xpi',
downloadProgress: 0,
status: UNINSTALLED,
needsRestart: false,
},
});
});

it('passes down needsRestart=true', () => {
assert.deepEqual(
installations(undefined, {
type: INSTALL_STATE,
payload: {
guid: 'my-addon@me.com',
url: 'https://cdn.amo/download/my-addon.xpi',
status: UNINSTALLING,
needsRestart: true,
},
}),
{
'my-addon@me.com': {
guid: 'my-addon@me.com',
url: 'https://cdn.amo/download/my-addon.xpi',
downloadProgress: 0,
status: UNINSTALLING,
needsRestart: true,
},
});
});
Expand All @@ -61,6 +84,7 @@ describe('installations reducer', () => {
url: undefined,
downloadProgress: 0,
status: INSTALLED,
needsRestart: false,
},
});
});
Expand All @@ -80,6 +104,7 @@ describe('installations reducer', () => {
url: undefined,
downloadProgress: 0,
status: UNINSTALLED,
needsRestart: false,
},
});
});
Expand All @@ -100,6 +125,7 @@ describe('installations reducer', () => {
url: 'https://cdn.amo/download/an-addon.xpi',
downloadProgress: 0,
status: INSTALLED,
needsRestart: false,
},
});
});
Expand Down