Simplified the building of RichAttributionWidget
#1661
Conversation
…m the render tree. While there was an assertion to check for this, it being removed ~1-2 frames after its added is perfectly valid, and thus we should easily check if the widget is still mounted. While we are at it, it seemed a GlobalKey was being used to track the context. In later versions of flutter Stateful widgets have a context property for this purpose.
Previous the widget waited two frames to find the dimensions it was allowed, then set the minWidth to be the width it naturally wanted. I think this was to ensure the widget took the full width of the screen. It was simple to set a Container's width to infinity to achive the same thing, with less complexity.
JaffaKetchup
changed the title
RichAttributionWidget
|
Yeah, I remember adding this and thinking it was less than ideal. It's quite possible that this approach was needed before, but with some of the changes since then, it's become redundant. |
There was a problem hiding this comment.
This has broken the sizing rules of the attribution box. On wider screens, it should shrink wrap around its contents, and on narrower screens, it should consume all of the available width. This appears to have made the box always behave as if it were on a narrow screen.
|
Can you provide me an example. Here are two screenshots (narrow and wide) from before my change: 
Both show the box expanding across the screen. Do you see something different? I'm happy to adjust to meet whatever you think is correct. |
|
Yep, so the wider screen should shrink wrap around the attribution text (be as small as possible), instead of expanding. You can check the latest master (latest exe build on master if you're on windows), to see what it should be. |
|
Those screenshots were taken from the latest master (without my change). Running the example app, in a Chrome browser. That's why I'm confused as I don't see what you see. |
|
Ah, that's strange, I checked latest main and it was different. Can you just run from a prebuild if that's possible, just to rule out any caching or something? I'll have a double check as well ASAP, but away from my PC atm. Edit: it may also not be wide enough. |
|
Ok using release from https://github.com/fleaflet/flutter_map/releases/tag/v6.0.0-dev.3 I get the behaviour you expect. After a lot of hitting vscode, the master branch now also looks like that. So I think I had some build caching issue somewhere :( Anyways, I'll make it act like how you want, which should take less time than debugging why my setup looked different to yours :) |
|
Minor tweak. It now seems to do what you would like. Please take a look. |
JaffaKetchup
changed the title
RichAttributionWidgetRichAttributionWidget
|
I'm happy to tweak it more if needed, to fill width if less than 420 pixel, otherwise its natural size. As for changelog/attribution I don't mind. I only stumbled on this because it caused a crash in my app, and happy to help improve the project. |
Previously the widget waited two frames to find the dimensions it was allowed, then set the minWidth to be the width it naturally wanted. I think this was to ensure the widget took the full width of the screen. It was simple to set a Container's width to infinity to achieve the same thing, with less complexity.
I may have misunderstood the purpose, but either way it seems I get the same behaviour when tested.