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: Resolve App details image duplications #290

Merged

Conversation

ss-nikunj
Copy link
Contributor

Description

  • The user uploads one image on the App details page while registering the app but it ended up showing the same image 3 times. Since max 3 images can be uploaded, the system somehow repeats the image thrice.
  • One single image repeat thrice on the Validate & Publish screen as well as on the App overview page.

Why

  • Image should display only once on both the Validate & Publish screen as well as on the App overview page.

Issue

  • Shared Component: #288
  • Portal Frontend: #1031

Checklist

  • I have followed the contributing guidelines
  • I have performed IP checks for added or updated 3rd party libraries
  • I have created and linked IP issues or requested their creation by a committer
  • I have performed a self-review of my own code
  • I have successfully tested my changes locally
  • I have added tests that prove my changes work
  • I have checked that new and existing tests pass locally with my changes
  • I have commented my code, particularly in hard-to-understand areas

image

@ss-nikunj ss-nikunj requested review from oyo and lavanya-bmw August 29, 2024 05:44
@ybidois
Copy link

ybidois commented Sep 9, 2024

Hi @oyo @lavanya-bmw, can you review this when you can? It's really a terrible user experience for the marketplace. Thanks!

const getIsInfinite = () => {
if (mobile) return gallery?.length > 1
else if (tab) return gallery?.length > 2
else return gallery?.length > 3
Copy link
Contributor

Choose a reason for hiding this comment

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

please remove else in both lines above because it's not needed when return in the if clause is guaranteed

CHANGELOG.md Outdated
@@ -12,6 +12,7 @@
- **chor:** run pretty explicitly ([702a9dc](https://github.com/eclipse-tractusx/portal-shared-components/commit/702a9dc7ca0ef003593ec5ad0ddf8459d92320f2))
- **chor:** update deoendency file ([bb45f76](https://github.com/eclipse-tractusx/portal-shared-components/commit/bb45f761102e442fd727b10f5a1d0362a0f124cf))
- **chro:** update the package version ([506dc70](https://github.com/eclipse-tractusx/portal-shared-components/commit/506dc709a28e19cbfc2dd6ea38f3af60ed3ef74c))
- **ImageGallery:** Resolve duplicates image issue ([#288](https://github.com/eclipse-tractusx/portal-shared-components/issues/288))
Copy link
Contributor

@oyo oyo Sep 11, 2024

Choose a reason for hiding this comment

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

remove entry because release-please will generate one based on your commit messages

Copy link

Choose a reason for hiding this comment

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

@oyo so we don't need to update the change log in shared components, only in the other repos. Do you confirm? Just to pass the knowledge to other FE devs.

Choose a reason for hiding this comment

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

@evegufy fyi

Copy link

@ss-nikunj
Copy link
Contributor Author

Hi @oyo,
As per your suggestions, I have removed the changelog entry and the redundant else statement from the code. Can you please review it again?

Thank you! 😊

@ss-nikunj ss-nikunj requested a review from oyo September 12, 2024 06:20
@MaximilianHauer MaximilianHauer added this to the Release 24.12 milestone Sep 13, 2024
Copy link
Contributor

@manojava-gk manojava-gk left a comment

Choose a reason for hiding this comment

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

lftm

@oyo oyo merged commit f39c355 into eclipse-tractusx:main Sep 13, 2024
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: USER READY
Development

Successfully merging this pull request may close these issues.

5 participants