Attribution is affected by map rotation

[Robbendebiene]

The attribution widget introduced by @ondbyte (#1012) gets rotated whenever the map is rotated.
Maybe attributionBuilder is supposed to be used in the nonRotatedLayers layer, but this means that the tiles of the TileLayerOptions won't be rotated as well.

I feel like the attributionBuilder in the TileLayerOptions is obsolete, since one can add any number of widgets over the map using children and nonRotatedChildren which is way more powerful.

[JaffaKetchup]

I was a little confused looking at the implementation of the attributionBuilder, but I haven't used it yet so I can't say much about it.

[github-actions]

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 5 days.

[github-actions]

This issue was closed because it has been stalled for 5 days with no activity.

[JaffaKetchup]

Can confirm this is an issue! @Robbendebiene Would be great if you could re-open it?

[Robbendebiene]

@JaffaKetchup I don't think that I can reopen it since the bot closed the issue. At least I don't see any button.

[JaffaKetchup]

I've reopened this so hopefully it can get solved at some point soon.

[Robbendebiene]

I would recommend removing this option, since this can easily be achieved with an Align widget inside the nonRotatedChildren, which is much more flexible.

[JaffaKetchup]

Not sure what's the best option here.
Most mapping packages have a built-in 'attributor', to make it less easy for the developer to forget to credit and therefore as a courtesy to map servers.
You could also just use a Stack for a homemade solution.
@ibrierley Any opinions?

[ibrierley]

I don't think we should get rid of it (just atm anyway), as it would be a breaking change for some. But there should be an added option or documentation about how to add a non-rotated one. Pull requests welcome!

[melis-m]

You could deprecate attributionBuilder, with a message explaining just that, if you don't want to get rid of it yet

[ibrierley]

I have no problem with it getting deprecated (I think), but I also think there should be something in place first to replace it. I suspect this will have to be a separate layer as suggested, as whole layers get rotated, and there's no real easy way for a tile layer to do this itself. It's a shame as it is intuitive to have the option as part of the tiles.

[JaffaKetchup]

Yeah I agree. I don't want to deprecate this without a replacement as it's quite a useful feature.

[ibrierley]

Theoretically in flutter_map_state.dart we could have a method..

List<Widget> _getAttributionLayers() {

    final List<Widget> attributionLayers = [];
    final layers = [...widget.layers, ...widget.children];
    for (final object in layers) {

      TileLayerOptions? tileOption;
      if (object is TileLayerOptions) {
        tileOption = object;
      } else if (object is TileLayerWidget) {
        tileOption = object.options;
      }

      if (tileOption != null && tileOption.attributionBuilder != null) {
        attributionLayers.add(
            Align(
                alignment: tileOption.attributionAlignment,
                child: tileOption.attributionBuilder!.call(context)
            )
        );
      }
    }
    return attributionLayers;
  }

then in _buildmap

final List<Widget> attributionLayers = _getAttributionLayers();

and in the nonRotatedChildren area have..

if(attributionLayers.isNotEmpty)
                ...attributionLayers,

Remove the bit of code from TileLayer.

I think this would work...
pros: It fixes the original problem and doesn't break or deprecate anything.
cons: it feels a bit clunky and is probably cluttering the map_state code, and not sure it's the most intuitive location.

[JaffaKetchup]

Personally that's a bit too clunky for me, and would just pollute the code in my opinion. However, I don't have another solution (other than just wrapping the FlutterMap() build method with a Stack around https://github.dev/fleaflet/flutter_map/blob/f6e2b230a2552d1953f0ba8980f5a9dadd47c32b/lib/src/map/flutter_map_state.dart#L76).

[ibrierley]

I don't think there's any nice way to do it without introducing a separate normal Widget layer. Another alternative then....

attribution_layer.dart

import 'package:flutter/widgets.dart';

class AttributionWidget extends StatelessWidget {
  final WidgetBuilder attributionBuilder;
  final Alignment alignment;

  const AttributionWidget({Key? key, required this.attributionBuilder, this.alignment =  Alignment.bottomRight})
      : super(key: key);

  @override
  Widget build(BuildContext context) {
    return Align(
        alignment: alignment,
        child: attributionBuilder.call(context)
    );
  }
}

flutter_map.dart

export 'package:flutter_map/src/layer/attribution_layer.dart';

Where you end up including flutter_map

nonRotatedChildren: [
                  AttributionWidget(
                    attributionBuilder: (_) {
                      return Text("© OpenStreetMap2 contributors");
                    }
                )],

Leave the original in and mark to be removed in future.

[JaffaKetchup]

Yeah that's probably the best option, I like it. It's small and still mostly self explanatory. Do you want to open PR or shall I?

[ibrierley]

I'm out now until tomorrow, so feel free.