Skip to content

Conversation

muffinresearch
Copy link
Contributor

@muffinresearch muffinresearch commented May 16, 2016

Fixes mozilla/addons#9587

Add the error message display into the Addon component. (It looks like this when enabled):

discover_add-ons

This could be a separate component which is composed into Addon. I'm not sure if that's necessary in reality though it would be easy enough to move out.

To use this does mean that the Addon component itself will need to become aware of the install state and supply the error message and close action as required. It would be expected that the close action would clear the state back to unknown and thus close the error overlay.

This doesn't yet do any transitioning, we'd want to look at using this [1] for that, and that should be a separate PR.

@mstriemer I'm happy to hear your thoughts on moving this around as needed. The component logic is very simple so re-jigging it would be easy.

[1] https://facebook.github.io/react/docs/animation.html

@coveralls
Copy link

Coverage Status

Coverage remained the same at 100.0% when pulling 93eff5b on muffinresearch:add-error-message-to-addon-component into 70015f0 on mozilla:master.

img {
.message {
flex-grow: 1;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops I'll kill this space.

@mstriemer
Copy link
Contributor

Looks good. Keeping it in Addon seems like the simplest thing and if we see a need to reuse this we can come up with a good way to extract this then.

Changes are good as they are, but I have bigger CSS concerns that I think we should address in another issue. Could merge as is or have some minor fix-ups, your call.

r+


&.theme {
display: block;
& .content .copy {
Copy link
Contributor Author

@muffinresearch muffinresearch May 17, 2016

Choose a reason for hiding this comment

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

The & is superfluous here - I'll remove it.

@muffinresearch muffinresearch force-pushed the add-error-message-to-addon-component branch from 93eff5b to 519cb41 Compare May 17, 2016 15:05
@muffinresearch muffinresearch force-pushed the add-error-message-to-addon-component branch from 519cb41 to 3ec7744 Compare May 17, 2016 15:09
@muffinresearch muffinresearch merged commit 598acc4 into mozilla:master May 17, 2016
@muffinresearch muffinresearch deleted the add-error-message-to-addon-component branch May 17, 2016 15:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Error message overlay
3 participants