Skip to content
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

Attribution ShadowDOM #306

Merged
merged 14 commits into from
Oct 5, 2022
Merged

Attribution ShadowDOM #306

merged 14 commits into from
Oct 5, 2022

Conversation

naogify
Copy link
Member

@naogify naogify commented Sep 30, 2022

Attribution Controlが外部のCSSの影響を受けてしまうので、 Attribution Controlのみを ShadowDOMとして追加しました。

最新版の Maplibre GL JS ではスマホで地図を表示した時、デフォルトでアトリビューションがオープンになるように変更があったので、それに併せて CSS を追加しました。

サンプル
https://codepen.io/naogify/pen/RwyMMrp

アトリビューションの open がデフォルトになった理由

ドラフトとして提出されている 新しい OSM の著作権ガイドラインに、ユーザーの操作なしに著作権表示をする項目が盛り込まれため、スマホ時のアトリビューションがデフォルトがオープンにする変更が加えられたとの事でした。

You may use a mechanism to fade/collapse the attribution under certain conditions:

・immediately with a dismiss interaction, for example clicking an 'x' in the corner of a dialog
・automatically on map interaction such as panning, clicking, or zooming
・automatically after five seconds. This also applies to splash screens or pop-ups.

https://wiki.osmfoundation.org/wiki/Licence/Attribution_Guidelines#Interactive_maps:~:text=You%20may%20use,or%20pop%2Dups

Issue
maplibre/maplibre-gl-js#205
PR
maplibre/maplibre-gl-js#795

@naogify
Copy link
Member Author

naogify commented Sep 30, 2022

@keichan34
地図を動かした時にアトリビューションを閉じる CSS も追加されていました、、。こちらレビューを少し待って頂ければと!

@naogify
Copy link
Member Author

naogify commented Sep 30, 2022

@keichan34
こちらの CSS の件、修正しました!

@naogify
Copy link
Member Author

naogify commented Oct 4, 2022

@keichan34
こちら © で改行しないように、css を追加しました。確認お願いします!

プレビュー
https://deploy-preview-306--geolonia-embed.netlify.app/debug.html

スクリーンショット 2022-10-04 12 22 01

Copy link
Member

@keichan34 keichan34 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

細かい修正ですがよろしくお願いします。
あと、テスト用のファイルを削除お願いします。(debug.html, style-long-attrb.json)

src/lib/util.js Outdated
@@ -274,3 +275,128 @@ export const handleRestrictedMode = (map) => {
container.classList.add('geolonia__restricted-mode-image-container');
}
};

/**
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maplibreから抽出したutilを別ファイルに分けましょう。maplibre-util.js とかかな。
もし将来このライブラリをTSに移行したら、簡単に元ソースを参照して使えるようにしたい


const style = document.createElement('style');
style.textContent = `
.mapboxgl-ctrl, .maplibregl-ctrl {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ここでは .mapbox-* 系の互換性のためのセレクタは不要です(shadow DOM内では完全にコントロールされているという環境なので、他のユーザーがこの中に何かしら入れたりすることはないし、外から触れることも無いので)

@keichan34
Copy link
Member

あ、ごめんなさい。©とその後の文言が改行しないようにCSSを指定したところをみせてもらってもいいですか?(なんか差分に埋もれてしまったような気がします。。。)(そして、debug.htmlのサンプルだとリンクになっていないので本番環境とは若干違うと思います。。

@naogify
Copy link
Member Author

naogify commented Oct 4, 2022

@keichan34
ありがとうございます! 上で指定して頂いた点を修正しました。 (後で、debug.html , style-long-attrb.json は削除します)
https://deploy-preview-306--geolonia-embed.netlify.app/debug.html

あ、ごめんなさい。©とその後の文言が改行しないようにCSSを指定したところをみせてもらってもいいですか?

こちらの件ですが下のように指定しています。(overflow-wrap: nowrap を指定したのですが、©️の直後に改行されてしまったので、 *1 word-break を使用しました。)

word-break: break-all;

@keichan34
Copy link
Member

なるほど。すごい細かいのですが、その実装だとこういうことが起こりうるんですね。この場合は「OpenStreetMap」などの一つの文言は分けたくないなと思って、 break-all が不適切かなと思います。

Screenshot from 2022-10-04 16-09-55

変わりに、それぞれのattributionを別のspanに入れて、そのspan自体がブレークしないようにできないかな。display: inline-block とか?

@naogify
Copy link
Member Author

naogify commented Oct 4, 2022

@keichan34

の場合は「OpenStreetMap」などの一つの文言は分けたくないなと思って

これは確かにですね、、、。

下のように指定し直しました。どうでしょうか?
https://github.com/geolonia/embed/blob/attribution-shadowdom/src/lib/CustomAttributionControl.js#L162

スクリーンショット 2022-10-04 16 20 24

@keichan34
Copy link
Member

なるほど。これでいいですね。ありがとうございました!

Copy link
Member

@keichan34 keichan34 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ありがとうございます!

Copy link
Member

@keichan34 keichan34 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

おっと、アプルーブするの忘れた。ありがとうございました!

@keichan34 keichan34 merged commit eba5f22 into master Oct 5, 2022
@keichan34 keichan34 deleted the attribution-shadowdom branch October 5, 2022 02:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants