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 for rendering HTML based labels #2856

Closed
wants to merge 3 commits into
base: master
from

Conversation

Projects
None yet
9 participants
@NathanW2
Member

NathanW2 commented Mar 1, 2016

This PR adds support for reading HTML based labels

image

Opened for review

Things to complete:

[ ] Tests
[X] Fix position
[ ] Disable options not supported with HTML labels

@luipir

This comment has been minimized.

Show comment
Hide comment
@luipir

luipir Mar 1, 2016

Hi Nathan, wonderful feature... do you mind what happen if it is passed a not well format html?ç

http://doc.qt.io/qt-4.8/qtextdocument.html#setHtml say that it's up to user... would be better to parse it's value in a dom document to check validity before 'accept" it? => some control in the label widget.
I understand this check wouldn't be done during label render providing due to processing overhead.

cheers

luipir commented on src/core/qgspallabeling.cpp in 99fb388 Mar 1, 2016

Hi Nathan, wonderful feature... do you mind what happen if it is passed a not well format html?ç

http://doc.qt.io/qt-4.8/qtextdocument.html#setHtml say that it's up to user... would be better to parse it's value in a dom document to check validity before 'accept" it? => some control in the label widget.
I understand this check wouldn't be done during label render providing due to processing overhead.

cheers

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 1, 2016

Owner

There is no issue setting invalid HTML you just don't get the nice rendered output. I don't see any need to validate it at this point unless it becomes an issue

I will be looking at ways to improve the expression builder widget to better display the results. I'm not really happy with the current results bit at the bottom.

Owner

NathanW2 replied Mar 1, 2016

There is no issue setting invalid HTML you just don't get the nice rendered output. I don't see any need to validate it at this point unless it becomes an issue

I will be looking at ways to improve the expression builder widget to better display the results. I'm not really happy with the current results bit at the bottom.

This comment has been minimized.

Show comment
Hide comment
@luipir

luipir replied Mar 1, 2016

ok

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Mar 1, 2016

Member

🎉

Member

m-kuhn commented Mar 1, 2016

🎉

@luipir

This comment has been minimized.

Show comment
Hide comment
@luipir

luipir Mar 1, 2016

Contributor

do you have tests in case of malformed html? what would be the expected user experience?

  • block rendering and notify error?
  • show warning for each lable? (label can be dynamic with user values => create a malformed html?)
  • show worning only the first malformed label and sto rendering label?
  • silently avoid to render malformed html?

sorry these are speculative questions... I'm not really a power user to decide what would be the better user experience.

btw... big advance in rendering :)

Contributor

luipir commented Mar 1, 2016

do you have tests in case of malformed html? what would be the expected user experience?

  • block rendering and notify error?
  • show warning for each lable? (label can be dynamic with user values => create a malformed html?)
  • show worning only the first malformed label and sto rendering label?
  • silently avoid to render malformed html?

sorry these are speculative questions... I'm not really a power user to decide what would be the better user experience.

btw... big advance in rendering :)

@luipir

This comment has been minimized.

Show comment
Hide comment
@luipir

luipir Mar 1, 2016

Contributor

another question... where this tags will be exported in QML and SLD? as CDATA?

Contributor

luipir commented Mar 1, 2016

another question... where this tags will be exported in QML and SLD? as CDATA?

@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 1, 2016

Member

@luipir there is no need for errors, etc. I just doesn't render correctly. Only a small subset of HTML is supported by QTextDocument so I don't see anything going wrong. Give this a try in a build if you have some ideas for testing bad data.

It will be exported/imported using the same method as composer labels are which support full html (even more html then this)

Member

NathanW2 commented Mar 1, 2016

@luipir there is no need for errors, etc. I just doesn't render correctly. Only a small subset of HTML is supported by QTextDocument so I don't see anything going wrong. Give this a try in a build if you have some ideas for testing bad data.

It will be exported/imported using the same method as composer labels are which support full html (even more html then this)

@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 1, 2016

Member

@luipir it gets stored as a string. Here is an example. This is just doing what we already do for labels.

<property key="labeling/fieldName" value="format('&lt;body>&#xd;&#xa;&lt;h1 style=&quot
Member

NathanW2 commented Mar 1, 2016

@luipir it gets stored as a string. Here is an example. This is just doing what we already do for labels.

<property key="labeling/fieldName" value="format('&lt;body>&#xd;&#xa;&lt;h1 style=&quot
@SrNetoChan

This comment has been minimized.

Show comment
Hide comment
@SrNetoChan

SrNetoChan Mar 1, 2016

Member

This is a great feature! Thanks @NathanW2 Will it respect the previous definitions made by the user? For instance, if the user choose to use a certain font and then apply a H1 tag, will it keep the chosse font type, or will it apply other font type?

Member

SrNetoChan commented Mar 1, 2016

This is a great feature! Thanks @NathanW2 Will it respect the previous definitions made by the user? For instance, if the user choose to use a certain font and then apply a H1 tag, will it keep the chosse font type, or will it apply other font type?

@3nids

This comment has been minimized.

Show comment
Hide comment
@3nids

3nids Mar 1, 2016

Member

you rock 👍

Member

3nids commented Mar 1, 2016

you rock 👍

@ghost

This comment has been minimized.

Show comment
Hide comment
@ghost

ghost Mar 1, 2016

Well, this is a very important labeling feature that users will truly appreciate for 2.16 ! Thanks Nathan !

ghost commented Mar 1, 2016

Well, this is a very important labeling feature that users will truly appreciate for 2.16 ! Thanks Nathan !

@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 1, 2016

Member

@3nids lets not go crazy now :)

Member

NathanW2 commented Mar 1, 2016

@3nids lets not go crazy now :)

@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 1, 2016

Member

@SrNetoChan Yeah that was the plan I think. @nyalldawson thought we could just build up a default style sheet and set it using defaultStyleSheet so that it will at least match what you have unless you override.

Member

NathanW2 commented Mar 1, 2016

@SrNetoChan Yeah that was the plan I think. @nyalldawson thought we could just build up a default style sheet and set it using defaultStyleSheet so that it will at least match what you have unless you override.

@SrNetoChan

This comment has been minimized.

Show comment
Hide comment
@SrNetoChan

SrNetoChan Mar 2, 2016

Member

Nice! Thanks for this. If you guys add this to the legend, I will be your fan forever! (Bah... I will already be your fan forever no matter what. )

Member

SrNetoChan commented Mar 2, 2016

Nice! Thanks for this. If you guys add this to the legend, I will be your fan forever! (Bah... I will already be your fan forever no matter what. )

@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 2, 2016

Member

@SrNetoChan :) What do you mean the legend?

Member

NathanW2 commented Mar 2, 2016

@SrNetoChan :) What do you mean the legend?

@SrNetoChan

This comment has been minimized.

Show comment
Hide comment
@SrNetoChan

SrNetoChan Mar 2, 2016

Member

I'm talking about the map composer legends. The ability to use HTML simple syntaxes, like bold and italic would be great. For instance, if the legend has species scientific names it's mandatory to have it in italic.

Member

SrNetoChan commented Mar 2, 2016

I'm talking about the map composer legends. The ability to use HTML simple syntaxes, like bold and italic would be great. For instance, if the legend has species scientific names it's mandatory to have it in italic.

@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 2, 2016

Member

Ah that is a cool idea. Maybe open a ticket for that.

Member

NathanW2 commented Mar 2, 2016

Ah that is a cool idea. Maybe open a ticket for that.

@klakar

This comment has been minimized.

Show comment
Hide comment
@klakar

klakar Mar 2, 2016

Really good!

If it would include Composer Custom Coordinate labels, I would be a very happy camper!

(Feature request #10301)

html_grid_labels

klakar commented Mar 2, 2016

Really good!

If it would include Composer Custom Coordinate labels, I would be a very happy camper!

(Feature request #10301)

html_grid_labels

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Mar 2, 2016

Member

@SrNetoChan Yeah that was the plan I think. @nyalldawson thought we could just build up a default style sheet and set it using defaultStyleSheet so that it will at least match what you have unless you override.

In combination with some kind of stylesheet management this will be incredibly flexibel!

Expression:

format('<div class="' + "style"+ '">' + "name" + '</div>')

Style definition:

.big { color: red; }
.normal { color: blue; }
.small { color: yellow; }

with features which have a style attribute of big, normal or small. Not sure if the qt html subset is powerful enough to support that... But hey!

Member

m-kuhn commented Mar 2, 2016

@SrNetoChan Yeah that was the plan I think. @nyalldawson thought we could just build up a default style sheet and set it using defaultStyleSheet so that it will at least match what you have unless you override.

In combination with some kind of stylesheet management this will be incredibly flexibel!

Expression:

format('<div class="' + "style"+ '">' + "name" + '</div>')

Style definition:

.big { color: red; }
.normal { color: blue; }
.small { color: yellow; }

with features which have a style attribute of big, normal or small. Not sure if the qt html subset is powerful enough to support that... But hey!

@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 2, 2016

Member

What ever qtextdocument supports we can do

On Wed, 2 Mar 2016 7:03 pm Matthias Kuhn notifications@github.com wrote:

@SrNetoChan https://github.com/SrNetoChan Yeah that was the plan I
think. @nyalldawson https://github.com/nyalldawson thought we could
just build up a default style sheet and set it using defaultStyleSheet so
that it will at least match what you have unless you override.

In combination with some kind of stylesheet management this will be
incredibly flexibel!

Expression:

format('<div class="' + "style"+ '">' + "name" + '')

Style definition:

.big { color: red; }
.normal { color: blue; }
.small { color: yellow; }

with features which have a style attribute of big, normal or small. Not
sure if the qt html subset is powerful enough to support that... But hey!


Reply to this email directly or view it on GitHub
#2856 (comment).

Member

NathanW2 commented Mar 2, 2016

What ever qtextdocument supports we can do

On Wed, 2 Mar 2016 7:03 pm Matthias Kuhn notifications@github.com wrote:

@SrNetoChan https://github.com/SrNetoChan Yeah that was the plan I
think. @nyalldawson https://github.com/nyalldawson thought we could
just build up a default style sheet and set it using defaultStyleSheet so
that it will at least match what you have unless you override.

In combination with some kind of stylesheet management this will be
incredibly flexibel!

Expression:

format('<div class="' + "style"+ '">' + "name" + '')

Style definition:

.big { color: red; }
.normal { color: blue; }
.small { color: yellow; }

with features which have a style attribute of big, normal or small. Not
sure if the qt html subset is powerful enough to support that... But hey!


Reply to this email directly or view it on GitHub
#2856 (comment).

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Mar 2, 2016

Member

What ever qtextdocument supports we can do

... if we have a stylesheet management 😜

Member

m-kuhn commented Mar 2, 2016

What ever qtextdocument supports we can do

... if we have a stylesheet management 😜

@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 2, 2016

Member

Just do this

format('<div class=' + @variable + '>' + "name" + '</div>')
Member

NathanW2 commented Mar 2, 2016

Just do this

format('<div class=' + @variable + '>' + "name" + '</div>')
@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Mar 2, 2016

Member

Is there already something in place to setup the css rules?

Member

m-kuhn commented Mar 2, 2016

Is there already something in place to setup the css rules?

@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 2, 2016

Member

Why not just use project variables. wouldn't that work just as well and be
less layers?

On Wed, Mar 2, 2016 at 8:23 PM, Matthias Kuhn notifications@github.com
wrote:

Is there already something in place to setup the css rules?


Reply to this email directly or view it on GitHub
#2856 (comment).

Member

NathanW2 commented Mar 2, 2016

Why not just use project variables. wouldn't that work just as well and be
less layers?

On Wed, Mar 2, 2016 at 8:23 PM, Matthias Kuhn notifications@github.com
wrote:

Is there already something in place to setup the css rules?


Reply to this email directly or view it on GitHub
#2856 (comment).

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Mar 2, 2016

Member

In your example the variable defines the class, but the rule for this class itself also needs to exist somewhere.

Member

m-kuhn commented Mar 2, 2016

In your example the variable defines the class, but the rule for this class itself also needs to exist somewhere.

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Mar 2, 2016

Member

Maybe like this:

format('<div class="' + "style" + '">' + "name" + '</div>', @project_css)
Member

m-kuhn commented Mar 2, 2016

Maybe like this:

format('<div class="' + "style" + '">' + "name" + '</div>', @project_css)
@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 Mar 2, 2016

Member

Ah right yeah sorry. Let me get the basics in first then we can go crazy ;)

On Wed, 2 Mar 2016 8:32 pm Matthias Kuhn notifications@github.com wrote:

Maybe like this:

format('<div class="' + "style" + '">' + "name" + '', @project_css)


Reply to this email directly or view it on GitHub
#2856 (comment).

Member

NathanW2 commented Mar 2, 2016

Ah right yeah sorry. Let me get the basics in first then we can go crazy ;)

On Wed, 2 Mar 2016 8:32 pm Matthias Kuhn notifications@github.com wrote:

Maybe like this:

format('<div class="' + "style" + '">' + "name" + '', @project_css)


Reply to this email directly or view it on GitHub
#2856 (comment).

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn Mar 2, 2016

Member

Sure, that will be an enhancement, no requirement. It's already incredibly useful the way it is.

Member

m-kuhn commented Mar 2, 2016

Sure, that will be an enhancement, no requirement. It's already incredibly useful the way it is.

@anitagraser

This comment has been minimized.

Show comment
Hide comment
@anitagraser

anitagraser Mar 20, 2016

Contributor

This is exciting 💯

Contributor

anitagraser commented Mar 20, 2016

This is exciting 💯

@m-kuhn

This comment has been minimized.

Show comment
Hide comment
@m-kuhn

m-kuhn May 13, 2016

Member

Merge it?

Member

m-kuhn commented May 13, 2016

Merge it?

@NathanW2

This comment has been minimized.

Show comment
Hide comment
@NathanW2

NathanW2 May 14, 2016

Member
Member

NathanW2 commented May 14, 2016

@NathanW2 NathanW2 added this to the QGIS 3 milestone Jul 8, 2016

@NathanW2 NathanW2 self-assigned this Jul 8, 2016

@nyalldawson

This comment has been minimized.

Show comment
Hide comment
@nyalldawson

nyalldawson Mar 24, 2017

Contributor

Closed after discussion with @NathanW2 . Needs someone else to pick this up and complete.

Contributor

nyalldawson commented Mar 24, 2017

Closed after discussion with @NathanW2 . Needs someone else to pick this up and complete.

@DelazJ

This comment has been minimized.

Show comment
Hide comment
@DelazJ

DelazJ Jun 11, 2018

Contributor

Closed after discussion with @NathanW2 . Needs someone else to pick this up and complete.

Can we now add the new "Worth a revival" label to this? I know it's old but I just needed the feature and remembered the excitement this PR raised years ago...

Contributor

DelazJ commented Jun 11, 2018

Closed after discussion with @NathanW2 . Needs someone else to pick this up and complete.

Can we now add the new "Worth a revival" label to this? I know it's old but I just needed the feature and remembered the excitement this PR raised years ago...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment