-
-
Notifications
You must be signed in to change notification settings - Fork 310
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
Use TextBox in InfoLayersSample and remove platform specific code to show info #2222
Conversation
I work on this because:
|
This PR removes the map info field in the platform specific UI and adds it to the InfoLayerSample, but there are other samples that use info as well. What to do with those? Below is a list of search for IsInfoLayer = true. I went through it, not all are important. Options:
|
After a bit of thinking I prefer the last option the most. User would have to explicitly add it, but the ShowMapInfoInMap option would probably be off by default, so it would also need: |
About building the MapInfoWidget. It would need to react to events all over the map so would have to be a transparent map sized widget, with the TextBox as a widget within the widget. |
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.
Nice improvement on maintainability
@@ -33,6 +35,18 @@ public static Map CreateMap() | |||
map.Layers.Add(new WritableLayer()); | |||
map.Layers.Add(CreateLineLayer()); | |||
|
|||
var textBox = CreateTextBox(); | |||
map.Widgets.Add(textBox); |
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.
Good idea to centralize the display of the mapinfo reduces the platform specific sample code and tests mapsui widgets.
textBox.Text = ""; | ||
else | ||
textBox.Text = $"Feature Info - {a.MapInfo.Feature.ToDisplayText()}"; | ||
map.RefreshGraphics(); |
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.
With this pull request changes on the widgets trigger automatically a redraw
I too would keep the MapControl slim. But I think there is a need for something like MapView where there is a default projection set and utility layers and widgets per default instantiated. Could have a different name than mapview. So on MapView there could be a showmapinfo property and on mapcontrol you add the mapinfowidget. |
The alternative I am thinking about would be a builder in which you could create such a component with one line of code. There could be different builders for different configurations. I am not sure yet if that would be sufficient replacement. |
…i/Mapsui into feature/use-textbox-in-info-sample
I also added a MapInfoWidget (and MapInfoWidgetRenderer), so this MR does not only changes the samples. I added it to the samples where I though it would be useful. map.Widgets.Add(new MapInfoWidget(map)); |
To show the data of a feature when using the Info event we had platform specific code. Not on all platforms, and with differences between platforms. This PR adds a TextBox in the InfoLayersSample so, that all the platform specific code could be removed.
I will test the changes platforms to check if I did not remove to much, or broke something otherwise.
The PR only affects the samples.