Esri / solutions-components

A collection of UI components for constructing webapps.
Apache License 2.0
11 stars 5 forks source link

Old manager app with hide map option enabled do not show the split view layout when changed in the configuration #848

Open azizaparveen opened 1 week ago

azizaparveen commented 1 week ago

Describe the bug

I am seeing this is issue only with this old app, where hide map was enabled, after the new code merge, I was able to change the layout option from the config but the default view is showing table view image

Expected Behavior

It shows Split view as default view since I changed the config option to Split view.

Reproduction Steps & Sample

Test app: https://sg-em-devext.mapsdevext.arcgis.com/apps/instant/manager/index.html?appid=d30ffa4fcdde411f8ced045a26d72030

  1. Launch the the app
  2. Notice that default view is Table whereas the config option for the layout has changed to split

I am also seeing issue in the configuration, if I changed the view to map view, it does not get updated in the preview, it does not load the map image

Other Relevant Info

No response

azizaparveen commented 1 week ago

image

sumitzarkar commented 1 week ago

@jmhauck For backward compatibility we are overriding appLayout to tableView when hideMapOnLoad is enabled

image

is this issue has impact on how this is handled in Instant app?

Please let us know your findings

jmhauck commented 3 days ago

@sumitzarkar At some point I feel like we should deprecate the prop from the component because it's now confusing what one would expect if they set both the hideMapOnLoad and appLayout.

For now at least I think we should just honor hideMapOnLoad if it's true and no appLayout is set. If appLayout is set then we should honor that and ignore hideMapOnLoad.

https://github.com/Esri/solutions-components/pull/867

@chris-fox do you have thoughts on the possible deprecation?

chris-fox commented 3 days ago

I am good with deprecating it. At this point I believe we are the only one using the component.

sumitzarkar commented 2 days ago

@jmhauck

867 changes looks good, after these changes please let us know if any other code changes are required from our side.