Skip to content
This repository has been archived by the owner on Jan 14, 2020. It is now read-only.

(DEV-4203) Fixes Prayer View on Small Phones #394

Merged
merged 3 commits into from
Nov 19, 2019
Merged

Conversation

redreceipt
Copy link
Contributor

@redreceipt redreceipt commented Nov 18, 2019

DESCRIPTION

Kapture 2019-11-18 at 14 25 35

Screen Shot 2019-11-18 at 2 31 52 PM

Screen Shot 2019-11-18 at 2 34 17 PM

What does this PR do, or why is it needed?

Simply wraps the bottom section of the prayer menu in a scroll view.

How do I test this PR?

Bring up the app on an iPhone 6s.

TODO

  • I am affirming this is my best work (Ecclesiastes 9:10)
  • PR has a relevant title that will be understandable in a public changelog (ie...non developers)
  • No new warnings in tests, in storybook, and in-app
  • Upload GIF(s) of iOS and Android if applicable
  • Set a relevant reviewer

REVIEW

  • Review updates to test coverage and snapshots
  • Review code through the lens of being concise, simple, and well-documented

Manual QA

  • Manual QA on iOS and ensure it looks/behaves as expected
  • Manual QA on Android and ensure it looks/behaves as expected

The purpose of PR Review is to improve the quality of the software.

@redreceipt redreceipt added the ready for review This is ready to be reviewd label Nov 18, 2019
@@ -16,10 +12,6 @@ import GET_CAMPUS_PRAYERS from '../data/queries/getCampusPrayers';
import GET_SAVED_PRAYERS from '../data/queries/getSavedPrayers';
import PrayerTab from './PrayerTab';

const StyledHorizontalTileFeed = styled({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this may cause problems but for the life of me I can't figure out why it was here...

@codecov-io
Copy link

codecov-io commented Nov 18, 2019

Codecov Report

❗ No coverage uploaded for pull request base (develop@521b140). Click here to learn what that means.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff            @@
##             develop    #394   +/-   ##
=========================================
  Coverage           ?   45.2%           
=========================================
  Files              ?     172           
  Lines              ?    1670           
  Branches           ?     183           
=========================================
  Hits               ?     755           
  Misses             ?     803           
  Partials           ?     112
Impacted Files Coverage Δ
...wspringchurchapp/src/prayer/PrayerTabView/index.js 15.38% <0%> (ø)
...ckages/newspringchurchapp/src/prayer/PrayerMenu.js 80% <66.66%> (ø)

@richarddubay
Copy link
Member

This is so much better. I don't mind the extra green space on the larger screens. Question: is it possible to scroll the whole thing and not just the bottom section? I expected the whole thing to move when I scrolled and not just the bottom part.

Admittedly, it doesn't have to scroll far, so it's not a giant deal on the iPhone 6s.

Also, I hope no one is trying to use our app on the iPhone SE (or 5s) but it does get a little wonky just scrolling the bottom half there:

Screenshot 2019-11-18 15 24 16

@redreceipt
Copy link
Contributor Author

This is so much better. I don't mind the extra green space on the larger screens. Question: is it possible to scroll the whole thing and not just the bottom section? I expected the whole thing to move when I scrolled and not just the bottom part.

Admittedly, it doesn't have to scroll far, so it's not a giant deal on the iPhone 6s.

Also, I hope no one is trying to use our app on the iPhone SE (or 5s) but it does get a little wonky just scrolling the bottom half there:

Screenshot 2019-11-18 15 24 16

may want to break that out into another PR, that is more complicated because the background color of that section is white

Copy link
Member

@richarddubay richarddubay left a comment

Choose a reason for hiding this comment

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

I'd like to come back to this at some point and see what we can do to make it better, but I think this solves the problem for now. Good work @redreceipt !

@richarddubay richarddubay merged commit 1e3ff6e into develop Nov 19, 2019
@richarddubay richarddubay deleted the scroll-prayers branch November 19, 2019 14:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ready for review This is ready to be reviewd
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants