Skip to content
This repository has been archived by the owner on Mar 5, 2020. It is now read-only.

better 'return to badge' styling, and a content switch on application… #2357

Merged
merged 3 commits into from Oct 20, 2016

Conversation

Pomax
Copy link
Contributor

@Pomax Pomax commented Oct 18, 2016

This centers the "return to badge" button in the application confirmation dialog, and also adds a context switch so that the modal cannot be closed until the POST request has properly finished - if the POST fails (not-200-return) the content is switch out for an error notice and a request to contact us, with a red button rather than yellow.

@cadecairos cadecairos temporarily deployed to learning-mozilla-org-s-pr-2357 October 18, 2016 22:33 Inactive
@Pomax Pomax mentioned this pull request Oct 18, 2016
11 tasks
@Pomax Pomax temporarily deployed to learning-mozilla-org-s-pr-2357 October 18, 2016 23:13 Inactive
Copy link
Contributor

@gideonthomas gideonthomas left a comment

Choose a reason for hiding this comment

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

lgtm


if (error) {
return (
<Modal className="modal-credly error folded" hideModal={this.state.canCloseModal? this.hideApplyModal : false}>
Copy link
Contributor

Choose a reason for hiding this comment

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

space before ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand? before what? and is there space, or should there be space?

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, I meant in the ternary condition, can we add a space between the condition and the ? operator?

@Pomax
Copy link
Contributor Author

Pomax commented Oct 19, 2016

added a space, although that's not a thing we;ve set up a js style rule for so atm either is "fine" from a linting perspective

@gideonthomas
Copy link
Contributor

feel free to merge in anytime

@Pomax Pomax merged commit 1f1b6e5 into master Oct 20, 2016
@Pomax Pomax deleted the return-to-badge branch October 20, 2016 05:18
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants