Open ElenaGallo opened 1 year ago
Thank you @ElenaGallo. @dsuren1 can you please investigate/estimate this?
Investigation
This is not a regression (also happens in v2022.02.00), as there was no implementation that specifically handles the removal of the query params when url generated for geostory-embedded
. By default all the query params applicable for shared url is present on embedded url.
The scroll information is removed from the shared url, only when Include scroll position
is set to false. This validation is unmodified
const shouldRemoveSectionId = !settings.showSectionId && advancedSettings && advancedSettings.sectionId;
let shareUrl = getSharedGeostoryUrl(removeQueryFromUrl(this.props.shareUrl), shouldRemoveSectionId);
We can add a specific config in embedOptions
or advancedSettings
perhaps to remove query params from shared url to form a embedded-url, there by handling this scenario in an isolated manner
localConfig:
{
geostory: [
{
"name": "Share",
"cfg": {
"embedOptions": {
"removeQueryFromShared": true
}
}
}
]
}
@tdipisa Kindly let me know your thoughts.
@dsuren1 thank you.
as there was no implementation that specifically handles the removal of the query params when url generated for geostory-embedded. By default all the query params applicable for shared url is present on embedded url.
I'm sorry but it is not clear to me. What do you mean precisely? The expected behavior in this case would not be to 'remove' the section path in case of embedded URL. The expected behavior would be to have it working by scrolling to the desired section also for embedded gestories.
As per the issue description, the expected result is
The URL of the embedded geostory does not contain the scroll position and the home button
As per the issue description, the expected result is
The URL of the embedded geostory does not contain the scroll position and the home button
@dsuren1 you are right, I'm sorry. I did't see it and probably I forgot that there is some missing implementation that is preventing the embedded URL to work with /section/id endpoint. I don't remember why precisely, we should check, but it is not the case here now since for this issue the identified behavior is effectively a regression from what we previously released: I suppose that's the reason why we decided remove the /section/id endpoint from the embedded URL. I can confirm that because I've tested with binaries of previous versions until 2021.01.03 where effectively the /section/id was not included in the embedded URL. The showHome query parmeter yes instead, but it is not preventing the embedded URL to work as expected: that parameter is simply ignored if present.
If you could spend 1h more to identify the reason of the regression providing also an estimate for the fix that would be good.
@tdipisa
From the gif, you can see that section id is not added when user is on the first section but added when scrolled to second. This is because the section id and column id is parsed from the url of the page here instead of the fetching the value from state->geostory-> currentPage
There are three issues,
Include scroll position
option enabledInclude scroll position
is enabled that adds section id to sharedUrlThis behavior exist on previous versions as well. This scenario can be easy to miss and probably overlooked and not happening just recently.
Description
The embedded geostory is not visible if the Advanced Options of the direct share link are enable.
How to reproduce
https://user-images.githubusercontent.com/56537133/226941975-fa26208f-9cd9-48a5-9cc3-5323b990b23a.mov
Expected Result
The URL of the embedded geostory does not contain the scroll position
Other useful information