Skip to content

BGDIINF_SB-2747: Add KML disclaimer icon#360

Closed
ltshb wants to merge 1 commit intodevelopfrom
feat-BGDIINF_SB-2747-kml-user-icon-2
Closed

BGDIINF_SB-2747: Add KML disclaimer icon#360
ltshb wants to merge 1 commit intodevelopfrom
feat-BGDIINF_SB-2747-kml-user-icon-2

Conversation

@ltshb
Copy link
Contributor

@ltshb ltshb commented Jan 26, 2023

TODO:

  • replace --URL-- in tooltip by the url.

Test link with external layer mf-geoadmin3 syntax
Test link

@ltshb ltshb force-pushed the feat-BGDIINF_SB-2747-kml-user-icon-2 branch 3 times, most recently from 5c158da to e875b58 Compare January 26, 2023 14:35
@ltshb ltshb force-pushed the feat-BGDIINF_SB-2747-kml-user-icon-2 branch from e875b58 to 7f6042c Compare January 26, 2023 15:53
@ltshb ltshb marked this pull request as ready for review January 26, 2023 15:53
@ltshb ltshb requested a review from pakb January 26, 2023 15:53
@ltshb
Copy link
Contributor Author

ltshb commented Jan 26, 2023

@pakb it is ready for a first review although I might change the tippy behavior to improve user experience next week.

Copy link
Contributor

@pakb pakb left a comment

Choose a reason for hiding this comment

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

looking good so far, a couple improvements / questions

getExternalDisclaimerPopupContent() {
return this.$i18n.t('external_data_tooltip')
getDisclaimerPopupContent() {
return this.$i18n.t('external_data_warning').replace('--URL--', this.AttributionName)
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this string specific to the new viewer? if so I would propose that we use some built-in formatting feature of VueI18N https://kazupon.github.io/vue-i18n/guide/formatting.html#named-formatting

meaning the message in the big spreadsheet can now be ... ... {url} ... ... and the call be this.$i18n.t('external_data_warning', { url: this.AttributionName)}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That would be great idea however this string is also used by the old viewer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think it would be better to duplicate this translation to use the new correct way ?
I propose to do a ticket for this, I would also fixed the following intlify warnings:

  • [intlify] Detected HTML in 'TESTSITE – DO NOT-SHARE - NOT FOR OPERATIONAL USE<br/>This site is for testing purposes only. It's not meant for operational use and there's no guarantee whatsoever.' message. Recommend not using HTML messages to avoid XSS.
  • shared.esm-bundler.js:54 [intlify] Detected HTML in 'Votre dessin est automatiquement stocké pour une année, vous acceptez ainsi les <a href='https://www.geo.admin.ch/fr/about-swiss-geoportal/impressum.html#drawing ' target='_blank'> conditions d'utilisation</a>.' message. Recommend not using HTML messages to avoid XSS.

Copy link
Contributor

@davidoesch davidoesch left a comment

Choose a reason for hiding this comment

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

Second link: Test link is ok ( it has tooltip in footer and in the logo)

@ltshb
Copy link
Contributor Author

ltshb commented Jan 31, 2023

Due to bad user experience with tooltip with long text on mobile, this PR is replaced by #362 which use a modal window on click with the big disclaimer text.
The main issue on mobile with long tooltip text, is that we have to set a timeout long enough for the reader to be able to read it and that if the reader is not interested on the tooltip or very quick reader, it has no intuitive way of closing the tooltip, clicking outside of it close it but also action any action that would do if it would have no tooltip. Therefore tooltip are good on mobile only for short text that could be read by everyone in less than 5 seconds.

@ltshb ltshb closed this Jan 31, 2023
@ltshb ltshb deleted the feat-BGDIINF_SB-2747-kml-user-icon-2 branch January 31, 2023 07:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants