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

WIP - Fix empty disclaimer #3195

Merged
merged 1 commit into from
Dec 5, 2017
Merged

WIP - Fix empty disclaimer #3195

merged 1 commit into from
Dec 5, 2017

Conversation

adube
Copy link
Contributor

@adube adube commented Dec 1, 2017

An issue has been raised about the disclaimer messages in the desktop_alt application. It's now showing an empty message. It seems to be a combinaison of multiple things. Here goes.

First issue

First, when the update to Angular 1.6 was made, changes were made in several directives to change them as components without them being tested. The gmf-disclaimer was one of them. Look at the first commit of this PR. The component was created before its controller. From this point moment on, the component stop from working under the 'development' environment. When compiled, it worked because I suppose the compiler was smart enough to understand to define the controller before and then use it, I don't know...

Introduced in #2142

Fixed in the "first step" commit.

Second issue

This PR doesn't have any fix for this yet, as I don't understand how it works...

The second issue was caused when moving the modal from a directive to a component, see:

#3154

Prior this, the modal was shown fine.

When not using the external option of the disclaimer, it's working fine (we have disclaimer messages correctly shown at the bottom left of the map). When using it (that's what the desktop_alt app does), it's showing the empty messages.

From what I understand, we define a ngeo-modal that uses a custom defined model that's shared between the modal and disclaimer... and the visibility as well. I don't understand how that's supposed to work now with components (nor how that was working before), as I'm not used to this kind of practice.

@fredj
Copy link
Member

fredj commented Dec 5, 2017

@adube I'm on the second issue

@fredj fredj closed this Dec 5, 2017
@fredj fredj reopened this Dec 5, 2017
@fredj
Copy link
Member

fredj commented Dec 5, 2017

@adube I've found the issue with gmf-modal pull request to come

@fredj fredj merged commit 42bf0a6 into master Dec 5, 2017
@fredj fredj deleted the 170-fix-empty-disclaimer branch December 5, 2017 14:04
@adube
Copy link
Contributor Author

adube commented Dec 5, 2017

@fredj great, thanks!

@fredj
Copy link
Member

fredj commented Dec 5, 2017

@adube see #3211 for the second issue, it was related to the transclude handling in ngeo-modal.

@fredj
Copy link
Member

fredj commented Dec 5, 2017

I suspect that the previous code in ngeo-modal was specially designed to confuse others devs (or scare small children)

@adube
Copy link
Contributor Author

adube commented Dec 5, 2017

Hahaha, it did scare me a little :)

@sbrunner sbrunner added this to the 2.3 milestone Apr 12, 2018
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