Added support for font images #9195
Conversation
| var styles = font.getStyles(); | ||
| styles['width'] = style.width; | ||
| styles['height'] = style.height; | ||
| styles['fontSize'] = (style.width > style.height ? style.height : style.width) + "px"; |
There was a problem hiding this comment.
qx.ui.table.cellrenderer.AbstractImage._getContentHtml() already adds "px" to the width/height, so the comparison can not work here and the "px" suffix is redundant
|
There should be an isFont-check added to https://github.com/cajus/qooxdoo/blob/e21fb220a5fd33dfce580bc5650e8336d255538e/framework/source/class/qx/bom/element/Decoration.js#L237 otherwise the warning is triggered on every font-icon |
| } | ||
|
|
||
| this.__updateContentHint(size, size); | ||
| el.setStyle("font-size", size + "px"); |
There was a problem hiding this comment.
Must be fontSize and the whole size-handling is also done in this.__setSource (which is called in line 749) so the size handling here might be redundant?
There was a problem hiding this comment.
Nearly redundant. There's some difference between the width/height handlings. Needs.
| if (this.getScale()) { | ||
| var width = this.getWidth() || this.getHeight() || 40; | ||
| var height = this.getHeight() || this.getWidth() || 40; | ||
| el.setStyle("font-size", (width > height ? height : width) + "px"); |
There was a problem hiding this comment.
That is automatically converted as far as I see.
| } | ||
| else { | ||
| var font = qx.theme.manager.Font.getInstance().resolve(this.getSource().match(/@([^/]+)/)[1]); | ||
| el.setStyle("font-size", font.getSize() + "px"); |
| var font = qx.theme.manager.Font.getInstance().resolve(source.match(/@([^/]+)/)[1]); | ||
| el.setStyles(font.getStyles()); | ||
| el.setStyle("display", "table-cell"); | ||
| el.setStyle("vertical-align", "middle"); |
| el.setStyles(font.getStyles()); | ||
| el.setStyle("display", "table-cell"); | ||
| el.setStyle("vertical-align", "middle"); | ||
| el.setStyle("text-align", "center"); |
| el.setStyle("text-align", "center"); | ||
|
|
||
| if (this.getScale()) { | ||
| el.setStyle("font-size", (this.__width > this.__height ? this.__height : this.__width) + "px"); |
|
|
||
| if (this.getScale()) { | ||
| el.setStyle("font-size", (this.__width > this.__height ? this.__height : this.__width) + "px"); | ||
| } |
There was a problem hiding this comment.
Also width/height must be set here, on order to override the values set by el.setStyles(font.getStyles()) in line 884
There was a problem hiding this comment.
That needs inspection.
| } | ||
| else { | ||
| el.setStyle("font-size", font.getSize() + "px"); | ||
| } |
There was a problem hiding this comment.
This else part is obsolete as the value has already been set in line 884 el.setStyles(font.getStyles())
| var ResourceManager = qx.util.ResourceManager.getInstance(); | ||
| var font = qx.theme.manager.Font.getInstance().resolve(source.match(/@([^/]+)/)[1]); | ||
|
|
||
| var styles = font.getStyles(); |
There was a problem hiding this comment.
The font styles map must be cloned otherwise the original map would be manipulated
There was a problem hiding this comment.
I'd consider this a bug. It should not be possible to modify internal stuff. I'd expect that map to be usable directly.
| _applyTextColor : function(value) | ||
| { | ||
| if (this.__getMode() === "font") { | ||
| if (value) { |
There was a problem hiding this comment.
I guess the content element should be retrieved by
var el = this.getContentElement();
if (this.__wrapper) {
el = el.getChild(0);
}
as it's done in e.g. _applyDecorator above.
| if (isFont) { | ||
| var ResourceManager = qx.util.ResourceManager.getInstance(); | ||
| var font = qx.theme.manager.Font.getInstance().resolve(source.match(/@([^/]+)/)[1]); | ||
| el.setStyles(font.getStyles()); |
There was a problem hiding this comment.
This might override styles already set by the Appearance (e.g. color), so it needs some checks here to prevent such behaviour although I currently do not know what would be the best strategy here.
This commit adds: * makes the generator inspect a given font and extract needed information about glyphs and their aspect ratio * support for managed webfonts in qx.ui.basic.Image * support for managed webfonts in qx.ui.table.cellrenderer.Image * extends the Image demo in the demobrowser * adds a new IconFont demo in the demobrowser * adds some documentation for using font images
|
Just squashed to get rid of the multiple commits. |
|
@cajus does this PR need existing apps to be changed if they do NOT want to use font images? |
|
Everything is like it was before, no change is required. |
|
OK then. I've walked over the code and could not find anything what seems to harm. Using font images has to wait as I've currently no time for this. |
There was a problem hiding this comment.
looks good, not tested it but all i can see is two minor typos. Looking forward to seeing this hit master, it's a great addition, well done @cajus
| Drawbacks | ||
| ========= | ||
|
|
||
| Only qx.ui.basic.Image and qx.ui.table.cellrendere.Image support icon |
There was a problem hiding this comment.
typo: qx.ui.table.cellrenderer.Image
| # http://qooxdoo.org | ||
| # | ||
| # Copyright: | ||
| # 2006-2010 1&1 Internet AG, Germany, http://www.1und1.de |
|
3 approvals, more than 2 business days |
This commit adds:
information about glyphs and their aspect ratio