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

Adjust gmf-drawfeature CSS styling #1024

Merged
merged 1 commit into from Apr 15, 2016

Conversation

adube
Copy link
Contributor

@adube adube commented Apr 12, 2016

Work in progress for the CSS styling of the gmf-drawfeature directive.

Todo

  • review
  • apply reviewer's required corrections
  • only show the list of features and export/delete all buttons when there's at least 1 feature
  • merge into one commit
  • travis build passes

The official icons won't be used as part of this PR. We'll wait the integration to the Desktop application to do so.

Live demo

@adube
Copy link
Contributor Author

adube commented Apr 12, 2016

@pgiraud If you want to have a quick "look and feel" go take a look at the live demo (see in the description above). You'll see how it looks with icons instead of text.

{{'Delete' | translate}}
</button>
</div>

<div class="gmf-eol"></div>
Copy link
Contributor

Choose a reason for hiding this comment

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

This sounds like a hack to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you suggest as alternative ?

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's go with this first. But we should replace this in the future. At least we'll try.

@adube adube force-pushed the gmf-drawfeature-css branch 3 times, most recently from 81df974 to 7bd2dd9 Compare April 13, 2016 19:50
@adube
Copy link
Contributor Author

adube commented Apr 13, 2016

@pgiraud Would you please do an official review of this task ? If everything is to your likings, then we could merge this. Thanks.

@adube adube changed the title (wip) gmf-drawfeature CSS styling Adjust gmf-drawfeature CSS styling Apr 13, 2016
@pgiraud
Copy link
Contributor

pgiraud commented Apr 15, 2016

Is it possible/easy to add tooltips to the draw buttons?

@pgiraud
Copy link
Contributor

pgiraud commented Apr 15, 2016

Sorry for my misunderstanding, but is the drawfeature.html partial intended to be overwritten in the gmf desktop app ? You're currently using fa-xxx classes in the partial. But you're providing gmf-icon-xxx classes at the same time.
I may be wrong but the partial should use the gmf-icon-xxx classes. And the example should make sure that an icon is correctly displayed no matter how.

@@ -8,6 +8,7 @@
<meta name="mobile-web-app-capable" content="yes">
<link rel="stylesheet" href="../../../node_modules/openlayers/css/ol.css" type="text/css">
<link rel="stylesheet" href="../../../node_modules/bootstrap/dist/css/bootstrap.css" type="text/css">
<link rel="stylesheet" href="../../../node_modules/font-awesome/css/font-awesome.css" type="text/css">
Copy link
Contributor

Choose a reason for hiding this comment

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

Weird indentation.

@pgiraud
Copy link
Contributor

pgiraud commented Apr 15, 2016

Here's what I meant in my previous comment:
9d6b0e8

@pgiraud
Copy link
Contributor

pgiraud commented Apr 15, 2016

I thought I already sent a comment about this, but can't find it.
I'd prefer you use .btn-link for the buttons like "list", "export (all)", "delete (all)".

@adube
Copy link
Contributor Author

adube commented Apr 15, 2016

Thanks for the review. I'll apply the required changes one by one, but I may need clarifications for some. I'll contact you if needed.

@adube
Copy link
Contributor Author

adube commented Apr 15, 2016

Corrections made. Live demo up to date. Ready for merge.

@pgiraud
Copy link
Contributor

pgiraud commented Apr 15, 2016

Looks good to me.

@pgiraud pgiraud merged commit 1d8c5c3 into camptocamp:master Apr 15, 2016
@adube adube deleted the gmf-drawfeature-css branch April 15, 2016 14:17
@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

3 participants