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

fix: list should have unique "key" in web application page #5502

Merged
merged 1 commit into from
Jan 22, 2025

Conversation

hongky-1994
Copy link
Contributor

@hongky-1994 hongky-1994 commented Jan 21, 2025

What this PR does:
Fix error log in console web

  • Child in a list should have unique "key" (Application list page)
    CleanShot 2025-01-21 at 16 43 59

Why we need it:

  • To make web console clean and prevent unwanted behavior

Which issue(s) this PR fixes:

  • None

Does this PR introduce a user-facing change?:
No

  • How are users affected by this change: None
  • Is this breaking change: No
  • How to migrate (if breaking change): None

Copy link

codecov bot commented Jan 21, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 26.29%. Comparing base (e0dfc18) to head (b1c1d1c).
Report is 4 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #5502   +/-   ##
=======================================
  Coverage   26.28%   26.29%           
=======================================
  Files         464      464           
  Lines       49624    49626    +2     
=======================================
+ Hits        13043    13047    +4     
+ Misses      35547    35546    -1     
+ Partials     1034     1033    -1     

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

@hongky-1994 hongky-1994 changed the title fix: remove error "unique key" in application page fix: resolve error in web console Jan 21, 2025
@hongky-1994 hongky-1994 changed the title fix: resolve error in web console fix: list should have unique "key" in web application page Jan 21, 2025
@hongky-1994 hongky-1994 marked this pull request as ready for review January 21, 2025 10:25
@hongky-1994 hongky-1994 force-pushed the fix/clear-warning-console branch from 72c046a to 77a2f53 Compare January 21, 2025 12:39
) : (
<>
<Fragment key={v.version}>
Copy link
Member

Choose a reason for hiding this comment

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

[ASK]
v.version is already used in L178. Is that OK?

(I'm sorry, but I'm not familiar with React)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for asking,

Yes, @t-kikuc it is the same key for 1 record when map from recentlyDeployment.versionsList.

The error show that we need uniq key for each child when map from recentlyDeployment.versionsList so I use v.version as uniq key.

Copy link
Member

Choose a reason for hiding this comment

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

@hongky-1994 Thank you! got it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you!
Is the v.version the v0.6.0 of the screenshot below?

スクリーンショット 2025-01-22 14 08 47

This column sometimes contains multiple artifacts and may contain different artifacts with the same version.
So, I think it's a bit risky to use v.version as the unique key.

スクリーンショット 2025-01-22 14 10 48

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank @Warashi !
I update the pull with key ${v.name}:${v.version}

image

The key is just needs to be unique in a single cell, so the name and version of artifacts in a single row must be different, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @hongky-1994 !

the name and version of artifacts in a single row must be different, right?

Yes, I think so. The combination of name and version is safe as a unique key in a single row.

t-kikuc
t-kikuc previously approved these changes Jan 22, 2025
Copy link
Contributor

@Warashi Warashi left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Member

@khanhtc1202 khanhtc1202 left a comment

Choose a reason for hiding this comment

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

👍

@khanhtc1202 khanhtc1202 merged commit b6fbc71 into master Jan 22, 2025
18 checks passed
@khanhtc1202 khanhtc1202 deleted the fix/clear-warning-console branch January 22, 2025 08:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants