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

update html tooltip to handle bootstrap 5 tabs #87

Merged
merged 3 commits into from
Sep 24, 2024

Conversation

mind84
Copy link
Contributor

@mind84 mind84 commented Sep 13, 2024

linked to this ticket

@mind84
Copy link
Contributor Author

mind84 commented Sep 13, 2024

@Gustry

I've tried this on Lizmap master branch and it works (tabs looks exactly like in the popup on the dnd_popup layer in Lizmap popup.qgs project, http://localhost:8130/index.php/view/map?repository=testsrepository&project=popup )

@nboisteault

for now this is not compatible with bootstrap 2, probably old maptip popup should be updated too to support new version of bootstrap.

@nboisteault nboisteault self-requested a review September 13, 2024 13:06
@mind84 mind84 marked this pull request as ready for review September 13, 2024 13:09
@Gustry
Copy link
Member

Gustry commented Sep 16, 2024

Can you please do first the PR on Lizmap plugin ? The test suite about tooltips is the other repository.
The file must be exactly similar, it's a strict copy/paste between the two repositories.

Two points to check :

  1. The LWC version must be provided in the HTTP request, and the tooltip code must be adapted accordingly (BS2 versus BS5), let's discuss how we can handle that.

for now this is not compatible with bootstrap 2, probably old maptip popup should be updated too to support new version of bootstrap.

  1. I don't know how we can handle that.
    If the HTML inside projects will be broken, then it's a kind of breaking change, so LWC 4.0 ?

@mind84
Copy link
Contributor Author

mind84 commented Sep 18, 2024

@Gustry, @nboisteault

The LWC version must be provided in the HTTP request

Maybe we can check lizmap_web_client_target_version metadata property? It's reliable? If that so, pass it along the http request to the server shouldn't be tricky but I need information on how to parse it. For example:

lizmap_web_client_target_version = 308000

I guess it means version 3.8.0

If it's lesser than X then render bootstrap 2 else b5...

If the HTML inside projects will be broken, then it's a kind of breaking change, so LWC 4.0 ?

Yes it will be a braking change, I don't know when b5 will be release

@Gustry
Copy link
Member

Gustry commented Sep 18, 2024

Maybe we can check lizmap_web_client_target_version metadata property? It's reliable?

No, it's not reliable, because users might have "dozens", "hundreds" of project on their server. We do not expect GIS technicians to update all projects stored on their server when they do an upgrade of Lizmap Web Client.

For instance, LWC 3.8.0 https://github.com/3liz/lizmap-web-client/releases/tag/3.8.0 can read CFG file targeting LWC 3.5, written in the "Requirements" :

QGIS project files targeting at least Lizmap Web Client 3.5.0 to be displayed

lizmap_web_client_target_version = 308000

It's the same as PyQGIS API, all major.minor.patch written with 2 numbers. 30800, which is the same as 030800

Qgis.versionInt()
33411

for QGIS 3.34.11

That's why LWC needs to send it's version in the HTTP header/request I think.

Yes it will be a braking change, I don't know when b5 will be release

I guess you mean LWC with BS5 ? Yes, we will discuss that soon. We just want to release and communicate first with 3.8.1 coming soon.

@mind84
Copy link
Contributor Author

mind84 commented Sep 18, 2024

No, it's not reliable...

Ok, it's clear to me.

I guess you mean LWC with BS5 ?

Yes exactly

@Gustry Gustry force-pushed the bootstrap_5_tooltip branch from 41c16ef to 40cad6d Compare September 23, 2024 10:43
@Gustry Gustry force-pushed the bootstrap_5_tooltip branch from ab2b0eb to 472488d Compare September 23, 2024 12:50
@mind84 mind84 closed this Sep 23, 2024
@mind84 mind84 deleted the bootstrap_5_tooltip branch September 23, 2024 15:49
@mind84 mind84 restored the bootstrap_5_tooltip branch September 23, 2024 15:50
@Gustry Gustry reopened this Sep 23, 2024
@mind84
Copy link
Contributor Author

mind84 commented Sep 23, 2024

@Gustry,

sorry, deleted my local branch by mistake

@Gustry
Copy link
Member

Gustry commented Sep 23, 2024

Sorry, why closing ? Was-it a mistake ? This PR is still relevant.

@Gustry Gustry merged commit 8416128 into 3liz:master Sep 24, 2024
14 checks passed
@mind84 mind84 deleted the bootstrap_5_tooltip branch September 24, 2024 12:40
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.

3 participants