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

stretch background of left pane #1210

Merged
merged 1 commit into from Mar 29, 2021
Merged

stretch background of left pane #1210

merged 1 commit into from Mar 29, 2021

Conversation

sssoleileraaa
Copy link
Contributor

Description

Fixes #1209

Checklist

If these changes modify code paths involving cryptography, the opening of files in VMs or network (via the RPC service) traffic, Qubes testing in the staging environment is required. For fine tuning of the graphical user interface, testing in any environment in Qubes is required. Please check as applicable:

  • I have tested these changes in the appropriate Qubes environment
  • I do not have an appropriate Qubes OS workstation set up (the reviewer will need to test these changes)
  • These changes should not need testing in Qubes

If these changes add or remove files other than client code, the AppArmor profile may need to be updated. Please check as applicable:

  • I have updated the AppArmor profile
  • No update to the AppArmor profile is required for these changes
  • I don't know and would appreciate guidance

If these changes modify the database schema, you should include a database migration. Please check as applicable:

  • I have written a migration and upgraded a test database based on main and confirmed that the migration applies cleanly
  • I have written a migration but have not upgraded a test database based on main and would like the reviewer to do so
  • I need help writing a database migration
  • No database schema changes are needed

Copy link
Contributor

@kushaldas kushaldas left a comment

Choose a reason for hiding this comment

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

Do you have any resolution limit? The first screenshot is from main branch.

Screenshot from 2021-02-01 17-32-15

Next screenshot is from this PR.

Screenshot from 2021-02-01 17-33-42

@creviera is this the correct behavior? Maybe 2k was too much of a resolution.

@sssoleileraaa
Copy link
Contributor Author

oh interesting! great screenshots kushal - what you're seeing is also happening on the main branch and this PR should have fixed it. i'll look into what's going on here.

@sssoleileraaa sssoleileraaa moved this from Ready for Review to In Development in SecureDrop Team Board Feb 8, 2021
@eloquence
Copy link
Member

(Given current sprint priorities, moving to top of backlog for now.)

@eloquence eloquence moved this from In Development to Near Term - SD Workstation in SecureDrop Team Board Feb 18, 2021
@eloquence eloquence moved this from Near Term - SD Workstation to In Development in SecureDrop Team Board Mar 22, 2021
@sssoleileraaa sssoleileraaa requested a review from a team as a code owner March 23, 2021 01:37
@sssoleileraaa
Copy link
Contributor Author

Here's an update on my progress:

Before:
Screenshot from 2021-03-22 18-42-15

After:
Screenshot from 2021-03-22 18-41-45

In motion:
looks-good-until-next-slide-plz

Words:
You can see an example of how we can support transparent widgets on top of our branding barre in this PR by looking at the UserProfile widget. This will look nicer when we start adding more left pane widgets, but we have to make sure that the bg image isn't too noisy as we add more towards the darker hexes.

The remaining thing left to do is to fix this:

svg-needs-more-height

The easiest thing might be to just edit the SVG so that the viewbox is taller (right now it's at 884px, and perhaps we just want to make it the largest size we plan on supporting for the client). But I think with two more custom widgets we can have the look we're going for here AND support an enormous client window (it's just a bit more complicated to do). :]

@sssoleileraaa
Copy link
Contributor Author

Okay this is ready

Signed-off-by: Allie Crevier <allie@freedom.press>
@sssoleileraaa sssoleileraaa moved this from In Development to Ready for Review in SecureDrop Team Board Mar 26, 2021
@eloquence
Copy link
Member

Functionally this looks good to me! The scaling behavior is nicer even on smaller screen sizes (logo is always visible), and in portrait mode, it resolves #1209 for me as advertised.

Before

Screenshot_2021-03-25_17-57-35

After

Screenshot_2021-03-25_18-01-59

@emkll emkll moved this from Ready for Review to Under Review in SecureDrop Team Board Mar 29, 2021
Copy link
Contributor

@emkll emkll left a comment

Choose a reason for hiding this comment

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

LGTM, works well with 3840 vertical pixels
Screenshot_2021-03-29_12-45-38

@emkll emkll merged commit c33f18a into main Mar 29, 2021
SecureDrop Team Board automation moved this from Under Review to Done Mar 29, 2021
@emkll emkll deleted the fix-1209 branch March 29, 2021 16:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

scale left pane background image
4 participants