Skip to content

Conversation

@aayushsrivastava
Copy link
Contributor

@aayushsrivastava aayushsrivastava commented Jan 11, 2025

fixes: #2581

RCA:

  1. The ImageTable/CommitTable/ChartTable component gets re-mounted (unmounted and mounted again) when the select function is called. This leads to the loss of any local state in these components (including the active page).
  2. The select function modifies a state in the AssembleFreight component.
  3. The DiscoveryTable component (which wraps the ImageTable/CommitTable/ChartTable component) is a nested function inside AssembleFreight. Every time AssembleFreight rerenders due to a change in its state, the DiscoveryTable function is redeclared, changing its memory reference. React thinks the component has been replaced, unmounts the old Table, and mounts a new one.

This problem is also described here: https://stackoverflow.com/a/64211013

@aayushsrivastava aayushsrivastava requested a review from a team as a code owner January 11, 2025 18:37
@netlify
Copy link

netlify bot commented Jan 11, 2025

Deploy Preview for docs-kargo-io ready!

Name Link
🔨 Latest commit 7ff94ad
🔍 Latest deploy log https://app.netlify.com/sites/docs-kargo-io/deploys/67849a7ede43a600082e50c6
😎 Deploy Preview https://deploy-preview-3246.docs.kargo.io
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@aayushsrivastava aayushsrivastava marked this pull request as draft January 11, 2025 19:19
@aayushsrivastava aayushsrivastava marked this pull request as ready for review January 11, 2025 19:34
@aayushsrivastava
Copy link
Contributor Author

After fixing the original bug, I noticed that the page still resets to 1 when you change the type of artifact. The second commit fixes that issue by calculating and setting the expected page when the table first mounts.

@aayushsrivastava
Copy link
Contributor Author

Screen.Recording.2025-01-12.at.2.31.28.PM.mov

@aayushsrivastava aayushsrivastava changed the title fix(ui): selecting freight in assembly resets the view to page 1 fix(ui): freight assembly tables keep resetting to page 1 Jan 12, 2025
…sembleFreight component

Signed-off-by: Aayush Srivastava <aayushsrivastava.work@gmail.com>
…cted row when it is first mounted

Signed-off-by: Aayush Srivastava <aayushsrivastava.work@gmail.com>
Signed-off-by: Aayush Srivastava <aayushsrivastava.work@gmail.com>
@codecov
Copy link

codecov bot commented Jan 13, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 52.05%. Comparing base (32291db) to head (7ff94ad).
Report is 341 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3246   +/-   ##
=======================================
  Coverage   52.05%   52.05%           
=======================================
  Files         295      295           
  Lines       26695    26695           
=======================================
  Hits        13895    13895           
  Misses      12041    12041           
  Partials      759      759           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@aayushsrivastava
Copy link
Contributor Author

This PR is ready for review. FYI: @krancour

@krancour krancour requested a review from Marvin9 April 9, 2025 18:37
@krancour krancour added this to the v1.4.1 milestone Apr 9, 2025
@krancour krancour added kind/bug Something isn't working as intended; If unsure that something IS a bug, start a discussion instead area/ui Affects the UI priority/normal This is the priority for most work labels Apr 9, 2025
@krancour krancour modified the milestones: v1.4.1, v1.4.2, v1.4.3 Apr 10, 2025
@krancour krancour modified the milestones: v1.4.3, v1.4.4, v1.5.1 Apr 23, 2025
@Marvin9
Copy link
Contributor

Marvin9 commented May 16, 2025

Hello @aayushsrivastava sorry for the delay. This is in good shape. I wanted to append some commits for alternative approach than to calculate page by selected contents, but for some reason couldn't push to your fork's main after adding commits. So I have raised a new PR #4120 that includes your commits as well. Let me know if you have any questions. And thanks a lot for this fix

@Marvin9 Marvin9 closed this May 16, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/ui Affects the UI kind/bug Something isn't working as intended; If unsure that something IS a bug, start a discussion instead priority/normal This is the priority for most work

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(ui): selecting freight in assembly resets the view

3 participants