appsmithorg / appsmith

Platform to build admin panels, internal tools, and dashboards. Integrates with 25+ databases and any API.
https://www.appsmith.com
Apache License 2.0
34.55k stars 3.73k forks source link

[Bug]: Maps widget currently does not handle string values and need to be typecast to number #11128

Open ramsaptami opened 2 years ago

ramsaptami commented 2 years ago

Is there an existing issue for this?

Current Behavior

When Maps widget is bound to a selected row of a table widget that contains latitude and longitude values in string format, then Maps widget fails to derive a location pin.

LOOM DEMO

Workaround is to typecast as number as follows image

Steps To Reproduce

  1. Go to app at link https://release.app.appsmith.com/applications/620a06c4a68a2619dc8d0caa/pages/620a06c4a68a2619dc8d0cac/edit
  2. Observe that since latitude and longitude values are in string, the first map widget does not display a pin, whereas the second map widget that's been typecast displays location pin appropriately.

Environment

Production

Version

Cloud

Tooluloope commented 2 years ago

@dilippitchika I don't think this should be fixed, The Structure of the marker is this Array<{ "lat": "number Min: -90 Max: 90 Required", "long": "number Min: -180 Max: 180 Required", "title": "string", "color": "string" }>

A number is required, best that can happen would be to throw the validation error

riodeuno commented 2 years ago

@Tooluloope @dilippitchika I think we should assist the user by trying to typecast these strings into integers where ever possible. Small features like these should be helpful to our users. Do you see any issues with implementing this?

dilippitchika commented 2 years ago

We do that to some extent in a few widgets, for example chart widget, but don't do it everywhere across widgets. We can add it here, but we also need to think about other widgets.

Tooluloope commented 2 years ago

@riodeuno @dilippitchika This can be really tricky. One of the reasons we have issues with the defaultValue for select widget was that we were trying to help the users in typecasting. I feel it's better to be predictable, So the users know that this expects a Number and not a Stringified Number.

I think it's best to show adequate messages in the evaluation popup so the user is clear of what needs to be done.

riodeuno commented 2 years ago

@Tooluloope @dilippitchika I agree with both your points.

  1. @Tooluloope If we have a technical hurdle in supporting such automatic typecasting, let's figure this out by laying out the RCA
  2. @dilippitchika Agreed. From a DX perspective, it makes sense to support expected scenarios like these, and is something we should include in all scenarios if it exists in one scenario.