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

πŸ‘©β€πŸ”¬ [Investment Day] - Project detail - modal presentation #752

Merged
merged 2 commits into from
Jul 15, 2019

Conversation

dusi
Copy link
Contributor

@dusi dusi commented Jul 13, 2019

πŸ“² What

Fixes the jaggy animation when showing project screen modally.

πŸ€” Why

Hiding the status bar in landscape mode on iPad causes the modal presentation to cause a weird jump. This happens because we're hiding the status bar in landscape even though it's the default thing on iPhone.

πŸ‘€ See

Trello, screenshots, external resources?

Before πŸ› After πŸ¦‹
Screen Shot 2019-07-12 at 9 34 38 PM Screen Shot 2019-07-12 at 9 55 07 PM
(there should be no change) (there should be no change)
Screen Shot 2019-07-12 at 9 34 45 PM Screen Shot 2019-07-12 at 9 55 10 PM
(there should be no change) (there should be no change)
Screen Shot 2019-07-12 at 9 34 59 PM Screen Shot 2019-07-12 at 9 55 15 PM
(there should be no change) (there should be no change)
Screen Shot 2019-07-12 at 9 55 22 PM Screen Shot 2019-07-12 at 9 35 20 PM
the shadow cuts off the shadow goes edge to edge
Screen Shot 2019-07-12 at 9 38 06 PM Screen Shot 2019-07-12 at 9 53 08 PM
(there should be no change) (there should be no change)
Screen Shot 2019-07-12 at 9 38 11 PM Screen Shot 2019-07-12 at 9 53 16 PM
(there should be no change) (there should be no change)
Screen Shot 2019-07-12 at 9 38 41 PM Screen Shot 2019-07-12 at 9 54 17 PM
(there should be no change) (there should be no change)
Screen Shot 2019-07-12 at 9 38 48 PM Screen Shot 2019-07-12 at 9 54 24 PM
(there should be no change) (there should be no change)
Screen Shot 2019-07-12 at 9 40 09 PM Screen Shot 2019-07-12 at 9 56 38 PM
(there should be no change) (there should be no change)
Screen Shot 2019-07-12 at 9 40 21 PM Screen Shot 2019-07-12 at 9 57 12 PM
(hides status bar) (does not hide the status bar)
Before After
status-bar-bug-480 status-bar-fixed-480
(jumps the navigation bar) (fixed)

βœ… Acceptance criteria

  • Modal presentation works as previously
  • Modal presentation doesn't affect any push / modal from the project page

@dusi dusi requested a review from dnywh July 13, 2019 05:24
@@ -25,9 +25,6 @@ public protocol ProjectPamphletViewModelOutputs {
/// Emits a project that should be used to configure all children view controllers.
var configureChildViewControllersWithProject: Signal<(Project, RefTag?), Never> { get }

/// Return this value from the view's `prefersStatusBarHidden` method.
var prefersStatusBarHidden: Bool { get }
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems unused

@@ -37,11 +37,11 @@
</subviews>
<color key="backgroundColor" white="1" alpha="1" colorSpace="calibratedWhite"/>
<constraints>
<constraint firstItem="Bfz-Sg-Kdb" firstAttribute="trailing" secondItem="bdk-Xa-fm9" secondAttribute="trailing" id="3Dg-lk-oRa"/>
<constraint firstAttribute="trailing" secondItem="bdk-Xa-fm9" secondAttribute="trailing" id="3Dg-lk-oRa"/>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changes leading & trailing constraints to align to superview instead of previously safe areas

Copy link
Contributor

@dnywh dnywh left a comment

Choose a reason for hiding this comment

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

nice touch!

@dusi dusi merged commit e7de7da into master Jul 15, 2019
@dusi dusi deleted the project-pamphlet-disable-hiding-status-bar-in-landscape branch July 15, 2019 21:36
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