-
Notifications
You must be signed in to change notification settings - Fork 0
Add facets to technological attributes #118
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
Conversation
| - `ISIC_NEW_API_URL` to the absolute URL of the ISIC API root | ||
| - this must include a trailing slash | ||
| - e.g. this value may be `https://api-sandbox.isic-archive.com/api/v2/` for testing | ||
| - e.g. this value may be `https://api.isic-archive.com/api/v2/` for testing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
|
Hello @EkaterinaPoddubskaya and @danlamanna -- when I visit the pre-deploy page on an incognito (no history) window, and then select under "TBP TYPE" : "3D: XP", I see a lot of "Not found" messages pop up on my screen (see screenshot). Can either of you take a look what might be the cause? |
|
The reason is that we don't have lesion data coming from the server for some of the pictures |
|
Hello @EkaterinaPoddubskaya -- thanks for the quick investigation and reply! Much appreciated :) Since "lesion_id" is not a mandatory field (not all images are associated with a lesion), the UI should not consider that an error (Not Found). Is there a way to make this a "soft fail" (where the user will not see a message)? |
|
Hello @neuroelf ! Yes, implementing a soft fail is possible. I'll be looking into it tomorrow and will update you accordingly. |
|
Hello @danlamanna and @EkaterinaPoddubskaya -- you two probably know github quite a bit better (than I do, I mean)... Given that some files have been changed in both this and another PR, does github automatically refactor this by applying diffs, or is it important in which order I apply the PRs? Or maybe you, @EkaterinaPoddubskaya, have made the changes in the overlapping files (package.json and imagesFilters.js in sources/models) in both PRs? |
|
Otherwise, given this PR has only 4 files changed... Would it make sense to add these changes to the other PR (#121)? |
|
Hello @neuroelf, while the order of PRs might be important, it’s not a major issue since I can resolve any conflicts if they arise. However, I would recommend keeping PRs separate to maintain clear and focused logic. |
|
@EkaterinaPoddubskaya -- OK, I will then merge the PRs in numeric order (unless you suggest a different order). Can you let me know if/when the "soft fail" is (can be) implemented? I would really want to avoid gallery visitors seeing that error message :) |
|
@neuroelf, numeric order should work fine. The "soft fail" is already implemented, but I need some time to test it properly before pushing. My guess is you'll be able to see it in this branch by tomorrow morning :) |
Deploying isic-gallery with
|
| Latest commit: |
97ada7f
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://fc264d71.isic-gallery.pages.dev |
| Branch Preview URL: | https://add-technological-attributes.isic-gallery.pages.dev |
This PR resolves issue #117