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

Fix text overflow in Source Interface replies #6301

Merged
merged 2 commits into from Feb 23, 2022
Merged

Fix text overflow in Source Interface replies #6301

merged 2 commits into from Feb 23, 2022

Conversation

ghost
Copy link

@ghost ghost commented Feb 22, 2022

Status

Ready for review

Description of Changes

I couldn't find an issue for this. I hope it's alright if I fixed it. The issue is when long text is displayed in the "Read Replies" box, the text would overflow out of the box. This patch fixes it.

Testing

  1. make dev
  2. Go to http://localhost:8080.
  3. Click on "Log in".
  4. Enter the test codename for last source (3/3) in the test file.
  5. The long word no longer overflow out of the "Read replies" box with this patch.

Deployment

N/A

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

If you made changes to the system configuration:

If you added or removed a file deployed with the application:

  • I have updated AppArmor rules to include the change

If you made non-trivial code changes:

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

Choose one of the following:

  • I have opened a PR in the docs repo for these changes, or will do so later
  • I would appreciate help with the documentation
  • These changes do not require documentation

@ghost ghost self-requested a review as a code owner February 22, 2022 20:32
@codecov-commenter
Copy link

codecov-commenter commented Feb 22, 2022

Codecov Report

Merging #6301 (9ea0fbd) into develop (ecd6b2b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff            @@
##           develop    #6301   +/-   ##
========================================
  Coverage    84.07%   84.07%           
========================================
  Files           60       60           
  Lines         4208     4208           
  Branches       508      508           
========================================
  Hits          3538     3538           
  Misses         549      549           
  Partials       121      121           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ecd6b2b...9ea0fbd. Read the comment docs.

Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

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

Hi @zishanmirza, thanks for the PR! I tested it using your test plan and functionally it looks good to me.

Before:
Screenshot 2022-02-22 at 16-11-43 SecureDrop Protecting Journalists and Sources

After:
Screenshot 2022-02-22 at 16-12-32 SecureDrop Protecting Journalists and Sources

Besides the one inline comment, I have one small nitpick: could you edit the first line of your commit message to be active and specify what component is being fixed. Like, "Fix text overflow in Source Interface replies".

securedrop/sass/source.sass Outdated Show resolved Hide resolved
Zishan Mirza added 2 commits February 23, 2022 07:29
When long text is displayed in the "Read Replies" box, the text
would overflow out of the box. This patch fixes it.

Signed-off-by: Zishan Mirza <[email protected]>
Switched "word-wrap" to "overflow-wrap".

Signed-off-by: Zishan Mirza <[email protected]>
@ghost
Copy link
Author

ghost commented Feb 23, 2022

I have changed the commit message title.

@legoktm legoktm changed the title Text overflow fix Fix text overflow in Source Interface replies Feb 23, 2022
Copy link
Member

@legoktm legoktm left a comment

Choose a reason for hiding this comment

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

LGTM, thanks for the contribution!

@legoktm legoktm merged commit f2c9586 into freedomofpress:develop Feb 23, 2022
@zenmonkeykstop zenmonkeykstop mentioned this pull request Mar 17, 2022
34 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.

2 participants