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(service marketplace): fix detail images #1197

Open
wants to merge 12 commits into
base: main
Choose a base branch
from

Conversation

Usmanfee
Copy link
Contributor

@Usmanfee Usmanfee commented Oct 4, 2024

Description

Service marketplace and service marketplace detail page has hardcoded images are being displayed. The original image should be displayed incase of its existence otherwise default images would be displayed.

Changelog entry:

- **Service Marketplace**
  - replaced hardcoded images in service marketplace and detail [#1195](https://github.com/eclipse-tractusx/portal-frontend/issues/1195)

Solution

There was one parameter of leadPictureId was missing in response of api which actually contains the image info so with leadPictureId and now by having leadPictureId we are displaying actual images.

Issue

#1195

Checklist

  • I have performed a self-review of my own code
  • I have successfully tested my changes locally
  • I have updated CHANGELOG.md file

Copy link
Contributor

@shubhamv-ss shubhamv-ss left a comment

Choose a reason for hiding this comment

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

lgtm

@Usmanfee Usmanfee marked this pull request as ready for review October 10, 2024 08:24
@MaximilianHauer
Copy link

added prio due to dependency with backend development.
@ntruchsess as discussed please review.

CHANGELOG.md Outdated Show resolved Hide resolved
@Usmanfee Usmanfee force-pushed the fix/service-marketplace-hardcoded-images branch from 03ae3a1 to e66f13f Compare October 15, 2024 08:58
Copy link
Contributor

@oyo oyo left a comment

Choose a reason for hiding this comment

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

Too much custom image loading logic that already has been implemented. See comments about how to use that. Also please update DEPENDENCIES.

src/components/pages/ServiceAdminBoardDetail/index.tsx Outdated Show resolved Hide resolved
src/components/pages/ServiceAdminBoardDetail/index.tsx Outdated Show resolved Hide resolved
}
loadImages()
}
}, [services])
Copy link
Contributor

Choose a reason for hiding this comment

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

Better modify CardHorizontal in the shared components to additionally allow an <Image ...> instead of an image url then we could also use its built in loading logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@oyo I have introduced this change in shared-component here in this PR.

This change will introduce some breaking change in HorizontalCard component. I think we first need to review PR for the shared component and later this PR should be review, test and merge. right ?

@Usmanfee Usmanfee force-pushed the fix/service-marketplace-hardcoded-images branch from fee7636 to 3dfd6f7 Compare October 24, 2024 13:55
@evegufy evegufy changed the base branch from main to release/v2.3.0-RC2 October 24, 2024 18:38
Copy link
Contributor

@evegufy evegufy left a comment

Choose a reason for hiding this comment

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

Check for build is failing.

Please also remove the changes to changelog and add a note in the PR description.

@evegufy evegufy changed the base branch from release/v2.3.0-RC2 to release/v2.3.0-RC3 November 4, 2024 19:13
Copy link

sonarcloud bot commented Nov 5, 2024

@Usmanfee
Copy link
Contributor Author

Usmanfee commented Nov 5, 2024

@evegufy I have reverted the change of changelog file. Regarding build failure, failure is expected since I introduced new changes in shared component which is not yet merged and released so that is why it is failing with linting.

eclipse-tractusx/portal-shared-components#356
this above PR first need to be merged and released in order to fix build failure for this PR.

If you have any other suggestion or work around to address this blocker then please let me know! Thanks

@evegufy
Copy link
Contributor

evegufy commented Nov 7, 2024

@evegufy I have reverted the change of changelog file. Regarding build failure, failure is expected since I introduced new changes in shared component which is not yet merged and released so that is why it is failing with linting.

eclipse-tractusx/portal-shared-components#356 this above PR first need to be merged and released in order to fix build failure for this PR.

If you have any other suggestion or work around to address this blocker then please let me know! Thanks

Hi @Usmanfee ok understood, @oyo is working on the issue why the integration takes to long, just fyi eclipse-tractusx/portal-shared-components#361

@evegufy evegufy changed the title fix(UI): service marketplace and detail images fix(service marketplace): fix detail images Nov 7, 2024
@evegufy evegufy changed the base branch from release/v2.3.0-RC3 to release/v2.3.0-RC4 November 12, 2024 17:10
@evegufy evegufy changed the base branch from release/v2.3.0-RC4 to main November 13, 2024 13:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: IN PROGRESS
Development

Successfully merging this pull request may close these issues.

6 participants