Skip to content
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

Flash message fixes #1558

Merged
merged 9 commits into from
Feb 6, 2017
Merged

Conversation

heartsucker
Copy link
Contributor

Fixes #1526, #1525, and #1524.

Doesn't include adding the custom org name to one of the flashed messages.

Some of this was pending until after #1538 got merged. Y'know, not to pile too much into one PR.

@heartsucker
Copy link
Contributor Author

Also @ninavizz is going to want to check these for colors since I guessed for some since values weren't (aren't?) in the relevant issues.

Copy link
Contributor

@redshiftzero redshiftzero left a comment

Choose a reason for hiding this comment

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

Looks good! Mostly just some small grammar edits inline (sorry - I know that text was just lifted from the images in the issues - my bad for not correcting at that time).

"notification")
flash(Markup("""{svg}<div class="message"><strong>Success!</strong>
<p>Thank you for sourcing this information to us.
Please check-back later for replies. <a href="#codename-hint">
Copy link
Contributor

Choose a reason for hiding this comment

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

'check-back' -> 'check back'

"Thanks for submitting something to SecureDrop! Please check back later for replies.",
"notification")
flash(Markup("""{svg}<div class="message"><strong>Success!</strong>
<p>Thank you for sourcing this information to us.
Copy link
Contributor

Choose a reason for hiding this comment

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

'sourcing' -> 'sending'

msg = Markup("""<div class="icon">{svg}</div>
<div class="message"><strong>Important!</strong><br>
<p>Thank you for exiting your session! Please select "New
Identity" from the green Onion button in the Tor browser,
Copy link
Contributor

Choose a reason for hiding this comment

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

delete comma at end of line

<strong>Whew, it’s you! Now, the embarrassing part...</strong></br>
<p>Our servers experienced an unusual surge of new activity, when you last visited. This could
have been human activity, an automated attack, or just some random blip. To err on the side
of caution, we put a hold on sending all documents from that day, through to our journalists.</p>
Copy link
Contributor

Choose a reason for hiding this comment

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

'day, through' -> 'day through'

of caution, we put a hold on sending all documents from that day, through to our journalists.</p>

<p>Now that we know you’re really a human, though, we’ll get your previous submission into the
hands of a journalist, straight away. We’re sorry for the delay. Please do check-back again
Copy link
Contributor

Choose a reason for hiding this comment

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

'journalist, straight away' -> 'journalist straight away'

Copy link
Contributor

Choose a reason for hiding this comment

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

'check-back' -> 'check back'

def test_svg_bad_path(self):
with source.app.app_context():
with self.assertRaises(PathException):
util.svg('../../../../etc/hosts')
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

strong
color: #673466
p
color: #555555
Copy link
Contributor

Choose a reason for hiding this comment

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

It's probably best to store these hex color codes as variables in _variables.sass

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we add these colors anywhere else, I'd do it, I feel like for the moment these are specific enough to the single place they're used to leave them here. Also, according to @ninavizz 's mock ups, we need to have text and link colors change which is actually going to require a chunk of alterations to how we structure the html/css if we want it to make sense.

@redshiftzero
Copy link
Contributor

Ok, changes look good now. Merging...

@redshiftzero redshiftzero merged commit b83f219 into freedomofpress:develop Feb 6, 2017
@ninavizz
Copy link
Member

ninavizz commented Feb 6, 2017

Thanks for cc'ing me @heartsucker — still getting my chops sharpened with this whole Terminal/Git/Unix/etc stuff, to check PRs... so, it'll likely not be until mid-week at the earliest, that I'll be able to look at PRs on my own. Until then—to meltdowns and the ultra-appreciated encouragement and help from @fowlslegs! :)

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.

3 participants