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

Fix crash on first login & scrollToIndex crash #1031

Merged
merged 5 commits into from Jan 7, 2022
Merged

Conversation

shu8
Copy link
Member

@shu8 shu8 commented Sep 14, 2021

  • Use package-lock version 2

  • Fix crash on login (https://sentry.io/share/issue/1c0a39f2de8847f78a51d460f1534a06/) due to all timetable entries being null

  • Update timetable tests

  • Fix scrollToIndex violation (https://sentry.io/share/issue/f183014529bf4111bfd5274797a9e7ed/) due to the initialScrollIndex not being rendered yet

  • Change the behaviour of the auto-scroll to only scroll to the current day if you're viewing the current week. Previously, all weeks started at the current day (e.g., today is Tuesday, scroll to next week, and the first day was still Tuesday).

  • Remove trailing /redirect from redirect URI (it looks like it was added to fix something but it seems to work on Android and iOS without it, and the docs don't mention the need for it)

Tested on iOS and Android devices.

@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2021

Codecov Report

Merging #1031 (07d34e2) into master (4ca0b34) will increase coverage by 0.76%.
The diff coverage is 56.25%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1031      +/-   ##
==========================================
+ Coverage   34.82%   35.59%   +0.76%     
==========================================
  Files         159      161       +2     
  Lines        2435     2433       -2     
  Branches      645      676      +31     
==========================================
+ Hits          848      866      +18     
+ Misses       1572     1545      -27     
- Partials       15       22       +7     
Impacted Files Coverage Δ
commitlint.config.js 0.00% <0.00%> (ø)
redux/actions/userActions.ts 34.37% <0.00%> (ø)
...eens/Timetable/TimetableScreen/TimetableScreen.tsx 42.05% <0.00%> (-0.81%) ⬇️
lib/Logger.ts 50.00% <50.00%> (ø)
lib/AnalyticsManager.ts 100.00% <100.00%> (+6.66%) ⬆️
screens/Rooms/RoomsSearchScreen/SearchControl.tsx 5.97% <0.00%> (ø)
...omponents/Button/FloatingButton/FloatingButton.tsx 0.00% <0.00%> (ø)
...ms/RoomsFavouritesScreen/RoomsFavouritesScreen.tsx 12.50% <0.00%> (ø)
...ySpaces/StudySpaceDetailScreen/FavouriteButton.tsx 6.66% <0.00%> (ø)
... and 3 more

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 1951067...07d34e2. Read the comment docs.

@shu8
Copy link
Member Author

shu8 commented Jan 4, 2022

CI deployment seems to be broken:

image

I'll fix that in another PR, and for now try to push these changes OTA (when this is merged).

@shu8 shu8 changed the title Improvements/fixes for the new year Fix crash on first login Jan 4, 2022
This fixes the crash on login when you first visit the timetable screen.
This was added in
3d7cce6#diff-92485d835e426b7a4ef0a643b8c946ff79948ad6388ef0089a9ba6907ad6562dR62
but I can't get the login to work without removing it, and the docs seem
to say that isn't required. It's working on Ethan and my device with
this change now.
This was happening when initialScrollIndex > 0 and the cell hadn't
rendered yet.

We'll now try to re-scroll after a slight delay to allow the cell to
render.

This also changes the behaviour of auto-scrolling so that only if you
scroll into the current week will the page auto-scroll to the current
day. Previously, all weeks started at the current day (e.g., today is
Tuesday, scroll to next week, and the first day is still Tuesday).
@shu8 shu8 changed the title Fix crash on first login Fix crash on first login & scrollToIndex crash Jan 4, 2022
@shu8 shu8 marked this pull request as ready for review January 4, 2022 21:00
@shu8 shu8 requested a review from greenfrogs January 4, 2022 21:01
Copy link

@greenfrogs greenfrogs 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

@shu8 shu8 merged commit ff7aff5 into master Jan 7, 2022
@shu8 shu8 deleted the shu8/start-of-year-fixes branch January 7, 2022 20:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants