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

Added some changes to reduce the bubble size #218

Merged
merged 5 commits into from Jan 18, 2019

Conversation

nightwarriorftw
Copy link
Contributor

No description provided.

@nightwarriorftw
Copy link
Contributor Author

@eloquence Here's a small PR for reducing the bubble size . Feel free to suggest changes .
Before
securedrop-previous

After
securedrop-client-2

@heartsucker
Copy link
Contributor

I think this is something @ninavizz wouid need to comment on.

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.

hey @nightwarrior-xxx thanks for taking a look at this issue! I've dropped a comment inline for you to take a look at, let me know your thoughts

@@ -481,7 +481,7 @@ def __init__(self, message, align):
label.setStyleSheet(label.css + 'border-bottom-left-radius: 0px;')
layout.setContentsMargins(0, 0, 0, 0)
self.setLayout(layout)
self.setContentsMargins(0, 0, 0, 0)
self.setContentsMargins(38, 38, 38, 38)
Copy link
Contributor

Choose a reason for hiding this comment

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

The effect using setContentsMargins has is to increase the spacing between the various UI elements, i.e. for two messages you can compare the view in the image in the issue here with the image below from me trying out this PR:

screen shot 2019-01-07 at 4 08 51 pm

which is close, but not quite what we want. What we can do is use Qt's QSizePolicy, which you can read more about here. To fix this issue we'll then want to do two things:

  1. Specify a minimum height for each message bubble using CSS's min-height property.
  2. Set the size policies to indicate that we want messages to be able to expand vertically, but not horizontally.

Check out one way of doing this here: e0666c0. We should get something that looks like for conversations that fill and do not fill the view respectively:

screen shot 2019-01-07 at 5 47 47 pm

screen shot 2019-01-07 at 5 48 17 pm

Play around with this and let me know what you think!

@nightwarriorftw
Copy link
Contributor Author

nightwarriorftw commented Jan 14, 2019

@redshiftzero Please suggest changes. Here are some clips attached
test-4
test-5

Apply the QSizePolicy to the conversation view, not the widget
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, thanks @nightwarrior-xxx! (I added a small commit, hope that's cool)

ideally we would squash commits together to make the git history a bit nicer. but i'll approve this one for merge and you can give the git history rewriting a whirl on your next patch :-)

@redshiftzero redshiftzero merged commit 0154d77 into freedomofpress:master Jan 18, 2019
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

3 participants