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

Changed newsroom logo alt tag to 'Logo Image' (#4280) #4980

Conversation

DrGFreeman
Copy link
Contributor

@DrGFreeman DrGFreeman commented Nov 9, 2019

Status

Ready for review

Description of Changes

Related to #4280 (does not fix completely).

Changes proposed in this pull request:

Change the <alt> tag of the Newsroom logo image in the source application from SecureDrop to Logo Image.

Note that this change addresses item 1) of #4280 but does not implement the dynamic Newsroom name suggested in item 2) of #4280.

Testing

Run make dev and login to the Source Interface.

Use the brower's inspection feature to verify that the <alt> tag of the logo is set to Logo Image. Perform this verification on the index page as well as in the other pages accessible via the Source Interface.

Checklist

If you made changes to the server application code:

  • Linting (make lint) and tests (make test) pass in the development container

If you made changes to securedrop-admin:

  • Linting and tests (make -C admin test) pass in the admin development container

N/A.

If you made changes to the system configuration:

N/A.

If you made non-trivial code changes:

  • I have written a test plan and validated it for this PR

N/A.

If you made changes to documentation:

  • Doc linting (make docs-lint) passed locally

N/A.

If you added or updated a code dependency:

Choose one of the following:

  • I have performed a diff review and pasted the contents to the packaging wiki
  • I would like someone else to do the diff review

N/A.

Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! One improvement that could be made would be to make "Logo Image" an i18nable string, by wrapping it in a call to gettext().

securedrop/source_templates/base.html Outdated Show resolved Hide resolved
securedrop/source_templates/index.html Outdated Show resolved Hide resolved
@DrGFreeman
Copy link
Contributor Author

@zenmonkeykstop, changes implemented as per your change request.

Copy link
Contributor

@zenmonkeykstop zenmonkeykstop left a comment

Choose a reason for hiding this comment

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

i18n working - LGTM!

@zenmonkeykstop zenmonkeykstop merged commit 380166c into freedomofpress:develop Nov 11, 2019
@zenmonkeykstop zenmonkeykstop mentioned this pull request Nov 22, 2019
24 tasks
@rocodes rocodes mentioned this pull request Nov 27, 2019
35 tasks
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.

None yet

2 participants