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

Support modal disclaimers #1348

Merged
merged 3 commits into from
Jun 9, 2016
Merged

Conversation

adube
Copy link
Contributor

@adube adube commented Jun 7, 2016

This PR adds the possibility to display disclaimer messages as modals instead of alerts in the disclaimer service (ngeo). The gmf.disclaimer directive now has an option that allows to turn this feature on (defaults to off).

The gmf layer tree example has been modified to be able to show this in action using a url parameter.

Note: this PR currently includes the content of #1341, since it hasn't been merged yet.

Todo

Live demos

@adube
Copy link
Contributor Author

adube commented Jun 7, 2016

Ready for review, and I expect to have lots of comments.

*/
this.modal;

this.modal = this.modal !== undefined ? this.modal : false;
Copy link
Member

Choose a reason for hiding this comment

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

Please prefere this.modal = this.modal === true ? this.modal : false; for this test.
Else modal can be toto or null etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't understand what you mean. this.modal should be false by default.

Copy link

@tsauerwein tsauerwein Jun 8, 2016

Choose a reason for hiding this comment

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

If you set up the directive with something like ... gmf-disclaimer-modal="ctrl.map"..., this.modal will be a map object. This is why @ger-benjamin prefers to use this.modal = this.modal === true.

I would use two different variables. E.g. set up the scope with:

    scope: {
     'modalIn': '<?gmfDisclaimerModal', 

And then in your controller:

  /**
   * @type {boolean}
   * @export
   */
  this.modal = this['modalIn'] === true;

This way you are sure that this.modal always has the correct type.

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's a nice trick. I'll do this.

@ger-benjamin
Copy link
Member

ger-benjamin commented Jun 8, 2016

Result is strange on opening mutliple popups at the same time. Can you open an issue with that ?

@adube
Copy link
Contributor Author

adube commented Jun 8, 2016

Result is strange on opening mutliple popups at the same time. Can you open an issue with that ?

See: #1357

@adube
Copy link
Contributor Author

adube commented Jun 8, 2016

Corrections applied. Ready for merge.


/**
* @type {boolean}
* @export
*/
this.modal = modal;
this.modal = modal === 'true' ? true : false;
Copy link
Member

Choose a reason for hiding this comment

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

simpler with the exact same result...

this.modal = modal === 'true';

@sbrunner
Copy link
Member

sbrunner commented Jun 9, 2016

Just a micro comment also that's looks good :-)

@adube
Copy link
Contributor Author

adube commented Jun 9, 2016

Rebased onto master and tiny fixed applied.

@sbrunner
Copy link
Member

sbrunner commented Jun 9, 2016

Thanks :-)

@sbrunner sbrunner merged commit 7e130b1 into camptocamp:master Jun 9, 2016
@adube adube deleted the gmf-modal-disclaimer branch June 9, 2016 13:41
@sbrunner sbrunner added this to the Older milestone Aug 23, 2019
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

4 participants