Fixed replaced image in product page #24983
Conversation
with this small code trick ,we can change the replaced image to the newest one immediately the time after the image ajax replaced with the page refresh or reload.
|
Hello @NoZTurn! This is your first pull request on the PrestaShop project. Thank you, and welcome to this Open Source community! |
| @@ -494,7 +494,7 @@ | |||
| const imageElement = document.querySelector( | |||
| DropzoneMap.savedImage(newImage.image_id), | |||
| ); | |||
| imageElement.src = newImage.image_url; | |||
| imageElement.src = newImage.image_url + '?' + Math.random(); | |||
There was a problem hiding this comment.
What's the behavior behind this? It's because of browser's cache?
There was a problem hiding this comment.
yes, I think is the browser cache, you can force the browser to renew the new image url after the image uploaded.
There was a problem hiding this comment.
@PierreRambaud wdyt? I really appreciate this little trick
There was a problem hiding this comment.
If we want to do something properly we have to get the image checksum instead of a random number.
Using Math.random can return twice the same number.
There was a problem hiding this comment.
checksum is the best solution, if Math.random() may return a repeat number, we can also use new Date() and its getTime() function instead of Math.random(), because all times a different, so this can avoid the same number repeat.
There was a problem hiding this comment.
If the image is only load once, why not, otherwise, if it has to be always displayed, we should use the checksum method
There was a problem hiding this comment.
If the image is only load once, why not, otherwise, if it has to be always displayed, we should use the checksum method
Looks like it doesn't have to be always displayed, it's only on replace, in order to kick the browser's cache out which is faster, but maybe every API endpoints can just provide the URL with image checksum : ping @jolelievre
There was a problem hiding this comment.
If the image is only load once, why not, otherwise, if it has to be always displayed, we should use the checksum method
Looks like it doesn't have to be always displayed, it's only on replace, in order to kick the browser's cache out which is faster, but maybe every API endpoints can just provide the URL with image checksum : ping @jolelievre
agree with you, I come across that at product list page, the image is also not updated after image replacement because of the browser's cache, so maybe we have to makes some change at the image core API.
|
@PrestaShop/prestashop-core-developers As this PR is not a bug but an improvement, do we want it to target the 1.7.8.x branch? It's fast to merge and improve the new product page, but the new build is really close :/ |
checksum is the best solution,but us a different tail number is more simple, if Math.random() may return a repeat number, we can also use new Date() and its getTime() function instead of Math.random(), because all times a different, so this can avoid the same number repeat. I renew my pull request above.
admin-dev/themes/new-theme/js/pages/product/components/dropzone/Dropzone.vue
Outdated
Show resolved
Hide resolved
…e/Dropzone.vue Co-authored-by: GoT <PierreRambaud@users.noreply.github.com>
Progi1984
added
the
Waiting for QA
admin-dev/themes/new-theme/js/pages/product/components/dropzone/Dropzone.vue
Outdated
Show resolved
Hide resolved
NeOMakinG
removed
the
Waiting for QA
Progi1984
added
the
Waiting for QA
|
@MatShir so we need to focus develop on this PR? |
|
@Progi1984 @NeOMakinG, friendly reminder, in a beta phase to avoid new bugs and to stabilize the software, we don't accept improvements. Could you please transfer the contribution into develop please ? @NoZTurn, thanks a lot for the improvements. It is very appreciated, but it is not on the right branch 😄 |
Robin-Fischer-PS
removed
the
Waiting for QA
Robin-Fischer-PS
added
Waiting for author
|
@NoZTurn we need to do these changes on the develop branch. The rebase is a bit hardcore, so I would suggest you to cherry pick commits into a new PR based on develop, don't hesitate if you need help. I could do it, but it's your work and I don't wan't to steal it :) |
Questions Answers
Branch? develop
Description? with this small code trick ,we can change the replaced image to the newest one immediately the time after the image ajax replaced without the page refresh or reload.
Type? improvement
Category? BO
BC breaks? no
Deprecations? no
Fixed ticket? Fixes #{20035}.
How to test? Just clear your caches and refresh your product v2 page.
Possible impacts? small change, no other impact.
|
@MatShir @NeOMakinG thanks for your sugguestion, and I have submit this pull request to the develop branch here #25089. |
with this small code trick ,we can change the replaced image to the newest one immediately the time after the image ajax replaced without the page refresh or reload.
This change is