8000 NT-1587: Improve performance by Arkariang · Pull Request #1013 · kickstarter/android-oss · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

NT-1587: Improve performance #1013

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

Merged

Conversation

Arkariang
Copy link
Contributor
@Arkariang Arkariang commented Oct 14, 2020

📲 What

  • BEFORE : every time the amount was updated, the addOn item was re-builded to reflact that amount, by doing that the rx-chains where triggered again.
  • NOW: will keep the reference to the selected addOns. We will update the quantity of the items just once the continue button has been pressed.

🤔 Why

👀 See

  • Select many addOns from a list of 25 addOns
    many-addOns-performance
  • memory usage screenshoot

memory-consumption-with-change

| | |

📋 QA

  • ⚠️ in order to about the current timeout, if testing this PR, query for now shippingRules instead of shippingRulesExtended in case not digital, here what you need to change
    Screen Shot 2020-10-13 at 5 03 16 PM

  • For a digital addOns you can try this one with 31 addOns -> https://www.kickstarter.com/projects/cmdrfalcon/sierra-collectors-quest

  • Pledge to a project with many addOns

  • Update that pledge to add/remove addOns -> take a look into the button disabled if same selection

  • Update pledge to a digital reward

Story 📖

NT-1587

@@ -249,6 +249,9 @@ class BackingAddOnViewHolderViewModel {
this.quantity
.compose<Pair<Int, Reward>>(combineLatestPair(addOn))
.map { data -> Pair(data.first, data.second.id()) }
.distinctUntilChanged { item1, item2 ->
Copy link
Contributor
@leighdouglas leighdouglas Oct 14, 2020

Choose a reason for hiding this comment

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

Could we rename item1 and item2 to be more reflective of what the actual items are? Just to make it more clear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in this case item type is Pair(int, long), not sure what name give them for it to be clearer, what do you think it would be helpful here @leighdouglas ?

Copy link
Contributor
@jgsamudio jgsamudio left a comment

Choose a reason for hiding this comment

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

LGTM!

@Arkariang Arkariang merged commit 4a6bf4a into feature/NT-1382-Backing-Add-Ons Oct 14, 2020
@Arkariang Arkariang deleted the imartin/improve-performance branch October 14, 2020 20:57
Arkariang added a commit that referenced this pull request Oct 15, 2020
* NT-1231 Creator Perspective query + View your pledge updated (#895)
* NT-1141: Refactor existing queries (#897)
* NT-1294: RecyclerView structure for reward & Add-ons (#896)
* NT-1222:Retrieve Add-ons data from Graph (#899)
* NT- 1290: Add-ons UI in BackingFragment (#900)
* NT-1171: Manage Pledge View Backing Info updates (#902)
* NT-1290: Add-ons UI(with real data from staging) (#905)
* NT-1326: Pledge header ui just for rewards (#906)
* NT-1327: Header animation (#911)
* NT-1171: (Fixed) Manage Pledge View Backing Info updates (#912)
* NT-1326 : Add total amount back to the header (#914)
* NT-1326: Reward Title going two lines (#918)
* NT-1345: Bonus support validation fixes (#917)
* NT-1345: Bonus support validation fixes (#917)
* NT-1383: New add-ons screen (#923)
* NT-1384: Add-ons list (#925)
* NT-1384- AddOns query (#927)
* NT-1411: Creator crash fixed (#929)
* NT-1385: Add Ons Card UI  (#930)
* NT-1386: Stepper UI on Add-On card (#937)
* NT-1422 && NT-1402 && NT-1383 && NT-1385 && NT-1382: Fix rejected (#946)
* NT-1386:Rejected fix (#947)
* NT-1381: Pledge header design polish (#948)
* NT-1426: Android Manage Pledge View Design Polish (#949)
* NT-1380 Bonus support design polish (#950)
* NT-1344: Updated checkout with new mutation  (#951)
* NT-1344: Updated checkout with new mutation 
* NT- 1462&& NT-1463: UpdateBacking Mutation  (#952)
* NT-1462:Update pledge flow (#954)
* NT-1445: Selected Reward Tag  (#955)
* NT-1390: Previously selected reward logic (#956)
* NT-1440:Display Empty State when add-ons unavailable (#961)
* NT-1453:Maximum pledge string updates (#962)
* NT-1460:Update add ons selection (#959)
* NT-1417: Show a native alert during Edit Reward if changes would remove add ons (#965)
* NT-1387: Fix Add-Ons Available tag for Digital Reward (#970)
* NT-1387: Fix - Select AddOns for digital reward (#971)
* [NT-1453][NT-1344][NT-1399][NT-1509]: Fix Pledge with Digital addOns  (#974)
* [NT-1417][NT-1510]:Fix change reward flow (#975)
* NT-1442: Android Display an error state if add-ons fail to load (#980)
* NT-1516: Update the Bonus Support Base Amount (#981)
* NT-1534: Sold-out add-ons (#983)
* [NT-1453] Bonus Support Max Pledge String Fix (#987)
* NT-1442: Hide shipping selector on network error (#986)
* NT-1471 | NT-1539 - Prevent Add-ons Alert & Add Pledge Label (#991)
* NT-1549 : Choose another reward flow (#993)
* NT-1539: Unprompted Edit Rewards Alert (#994)
* NT-1541: Updated Add-ons Quantity Not Updated (#995)
* NT-1534: Android Sold-out add-ons fix (#996)
* NT-1534: Modify previously backed AddOns if unavailable (#1005)
* NT-606: FIX Display a string indicating backing state for creators on View/Manage Pledge screen 
* NT-1585:Reward with starting time restriction not started yet. (#1009)
* https://kickstarter.atlassian.net/browse/NT-1587 (#1011)
* NT- 1591:Blanck screen edit reward flow (#1012)
* NT-1531: Android Refactor RewardUtils Java to Kotlin (#1008)
* NT-1587: Improve performance (#1013)
* NT-1587: Query with filtering for shippingRules (#1015)

Co-authored-by: Jade Byfield <jjbyfield@gmail.com>
Co-authored-by: Jade Byfield <jade@thoughtbot.com>
Co-authored-by: leighdouglas <leighcdouglas1@gmail.com>
Co-authored-by: jgsamudio <jonathan2457@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0