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

gmf-displaywindow Component #3044

Merged
merged 1 commit into from
Nov 3, 2017
Merged

gmf-displaywindow Component #3044

merged 1 commit into from
Nov 3, 2017

Conversation

adube
Copy link
Contributor

@adube adube commented Oct 31, 2017

This PR includes a new component that can be used as replacement of the ngeo.Popup, but with the possibility to make it resizable/draggable.

It is used in all 4 apps, instead of the ngeo.Popup:

  • desktop
  • desktop_alt
  • mobile
  • mobile_alt

Todo

@adube
Copy link
Contributor Author

adube commented Oct 31, 2017

@fredj You can take an early look, if you want.

@adube adube changed the title WIP - gmf-displaywindow Component gmf-displaywindow Component Nov 1, 2017
@adube adube requested a review from fredj November 1, 2017 15:30
@adube
Copy link
Contributor Author

adube commented Nov 1, 2017

@fredj Ready for review. See also the "todo" about the desktop_alt. I don't know what's wrong...

width: calc(~"100% -" 2 * @app-margin);
z-index: @above-search-index;
}
}
Copy link
Member

Choose a reason for hiding this comment

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

new line is missing (see travis)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@adube
Copy link
Contributor Author

adube commented Nov 2, 2017

Travis fails, but it seems unrelated this PR.

@fredj
Copy link
Member

fredj commented Nov 2, 2017

I've tried to check what could be wrong with desktop_alt but without luck.
Maybe @romanzoller will had a clue

@adube
Copy link
Contributor Author

adube commented Nov 2, 2017

Rebased onto master.

@fredj
Copy link
Member

fredj commented Nov 3, 2017

The issue with desktop_alt is already present in master; not related to your changes.

Copy link
Member

@fredj fredj left a comment

Choose a reason for hiding this comment

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

Please open a new issue for desktop_alt and merge.

@adube
Copy link
Contributor Author

adube commented Nov 3, 2017

Please open a new issue for desktop_alt and merge.

Done: #3056. Merging...

@adube
Copy link
Contributor Author

adube commented Nov 3, 2017

Hold on. The issue reported by Travis is correct. I'll fix this first.

@adube
Copy link
Contributor Author

adube commented Nov 3, 2017

Error reported by Travis has been fixed. Remaining errors are unrelated to this PR. Merging...

@adube adube merged commit f9f2c01 into master Nov 3, 2017
@adube adube deleted the 122-gmf-displaywindow branch November 3, 2017 18:50
@sbrunner
Copy link
Member

sbrunner commented Nov 6, 2017

As I understand this will breaks the c2cgeoportal build...
https://travis-ci.org/camptocamp/c2cgeoportal/jobs/297853504#L2315

@adube
Copy link
Contributor Author

adube commented Nov 6, 2017

Why hasn't Travis report this in its last pass here? I'm concerned.

For the time being, I'll make a fix asap. It would be very, very nice I could be able to run the checks locally without any errors, or if Travis could behave normally every time...

@adube
Copy link
Contributor Author

adube commented Nov 6, 2017

Nevermind, this has already been fixed here: 542156f

@fredj
Copy link
Member

fredj commented Nov 6, 2017

Why hasn't Travis report this in its last pass here?

That's what I'm wondering too !

@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