You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@poi.apache.org by GitBox <gi...@apache.org> on 2022/12/23 08:25:19 UTC

[GitHub] [poi] gffloodg opened a new pull request, #409: Textruns not honouring highlight

gffloodg opened a new pull request, #409:
URL: https://github.com/apache/poi/pull/409

   This PR adds support for getting/setting the highlight color in TextRuns. It also adds support for rendering of the same. As usual, appropriate tests also added.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] gffloodg commented on a diff in pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
gffloodg commented on code in PR #409:
URL: https://github.com/apache/poi/pull/409#discussion_r1058339152


##########
poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java:
##########
@@ -158,6 +162,78 @@ private static void fetchFontColor(CTTextCharacterProperties props, Consumer<Pai
     }
 
 
+    /**
+     * Returns the font highlight (background) color for this text run.
+     * This returns a {@link SolidPaint}, or null if no highlight is set.
+     *
+     * @return The font highlight (background) colour associated with the run, null if no highlight.
+     *
+     * @see org.apache.poi.sl.draw.DrawPaint#getPaint(java.awt.Graphics2D, PaintStyle)
+     * @see SolidPaint#getSolidColor()
+     * @since POI 5.2.4
+     */
+    @Override
+    public PaintStyle getHighlightColor() {
+        XSLFShape shape = getParagraph().getParentShape();
+        final boolean hasPlaceholder = shape.getPlaceholder() != null;
+        return fetchCharacterProperty((props, val) -> fetchHighlightColor(props, val, shape, hasPlaceholder));

Review Comment:
   done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] pjfanning commented on a diff in pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #409:
URL: https://github.com/apache/poi/pull/409#discussion_r1057895126


##########
poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java:
##########
@@ -158,6 +162,78 @@ private static void fetchFontColor(CTTextCharacterProperties props, Consumer<Pai
     }
 
 
+    /**
+     * Returns the font highlight (background) color for this text run.
+     * This returns a {@link SolidPaint}, or null if no highlight is set.
+     *
+     * @return The font highlight (background) colour associated with the run, null if no highlight.
+     *
+     * @see org.apache.poi.sl.draw.DrawPaint#getPaint(java.awt.Graphics2D, PaintStyle)
+     * @see SolidPaint#getSolidColor()
+     * @since POI 5.2.4
+     */
+    @Override
+    public PaintStyle getHighlightColor() {
+        XSLFShape shape = getParagraph().getParentShape();
+        final boolean hasPlaceholder = shape.getPlaceholder() != null;
+        return fetchCharacterProperty((props, val) -> fetchHighlightColor(props, val, shape, hasPlaceholder));
+    }
+
+
+    private static void fetchHighlightColor(CTTextCharacterProperties props, Consumer<PaintStyle> val, XSLFShape shape, boolean hasPlaceholder) {
+        if (props == null) {
+            return;
+        }
+
+        final CTColor col = props.getHighlight();
+        if (col == null) {
+            return;
+        }
+
+        final CTSRgbColor rgbCol = col.getSrgbClr();
+        final byte[] cols = rgbCol.getVal();
+        final SolidPaint paint = DrawPaint.createSolidPaint(new Color(0xFF & cols[0], 0xFF & cols[1], 0xFF & cols[2]));
+        val.accept(paint);
+    }
+
+    /**
+     * Sets the font highlight (background) color for this text run - convenience function
+     *
+     * @param color The highlight (background) color to set.
+     * @since POI 5.2.4
+     */
+    @Override
+    public void setHighlightColor(final Color color) {
+        setHighlightColor(DrawPaint.createSolidPaint(color));
+    }
+
+    /**
+     * Set the highlight (background) color for this text run.
+     *
+     * @param color The highlight (background) color to set.
+     *
+     * @see org.apache.poi.sl.draw.DrawPaint#createSolidPaint(Color)
+     * @since POI 5.2.4
+     */
+    @Override
+    public void setHighlightColor(final PaintStyle color) {
+        if (!(color instanceof SolidPaint)) {
+            LOG.atWarn().log("Currently only SolidPaint is supported!");
+            return;

Review Comment:
   I'm not sure - can we make this an exception? IllegalArgumentException maybe. Needs to be in the javadoc, one way or another.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] asfgit closed pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #409: Textruns not honouring highlight
URL: https://github.com/apache/poi/pull/409


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] pjfanning commented on a diff in pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #409:
URL: https://github.com/apache/poi/pull/409#discussion_r1056202363


##########
poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextRun.java:
##########
@@ -440,6 +440,23 @@ public void setFontColor(PaintStyle color) {
         setFontColor(rgb);
     }
 
+
+    @Override
+    public PaintStyle getHighlightColor() {
+        return null;
+    }
+
+    @Override
+    public void setHighlightColor(final Color color) {

Review Comment:
   why add these methods? - if this stuff is only supported on XSLFTextRun, then only that class should have these methods.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] pjfanning commented on a diff in pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #409:
URL: https://github.com/apache/poi/pull/409#discussion_r1056201051


##########
poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java:
##########
@@ -157,6 +160,52 @@ private static void fetchFontColor(CTTextCharacterProperties props, Consumer<Pai
         }
     }
 
+    @Override

Review Comment:
   can you add javadoc on public API methods? include `@since POI 5.2.4`



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] gffloodg commented on a diff in pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
gffloodg commented on code in PR #409:
URL: https://github.com/apache/poi/pull/409#discussion_r1057764538


##########
poi/src/main/java/org/apache/poi/sl/usermodel/TextRun.java:
##########
@@ -79,6 +80,37 @@ enum FieldType {
     void setFontColor(PaintStyle color);
 
 
+    /**
+     * Returns the font highlight (background) color for this text run.
+     * This returns a {@link SolidPaint}, or null if no highlight is set.

Review Comment:
   Code updated, ready for review 



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] gffloodg commented on a diff in pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
gffloodg commented on code in PR #409:
URL: https://github.com/apache/poi/pull/409#discussion_r1056812245


##########
poi/src/main/java/org/apache/poi/sl/usermodel/TextRun.java:
##########
@@ -79,6 +80,37 @@ enum FieldType {
     void setFontColor(PaintStyle color);
 
 
+    /**
+     * Returns the font highlight (background) color for this text run.
+     * This returns a {@link SolidPaint}, or null if no highlight is set.

Review Comment:
   Ok sure, let me take a look and update.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] pjfanning commented on pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
pjfanning commented on PR #409:
URL: https://github.com/apache/poi/pull/409#issuecomment-1366819338

   Thanks - merged


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] gffloodg commented on a diff in pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
gffloodg commented on code in PR #409:
URL: https://github.com/apache/poi/pull/409#discussion_r1056419343


##########
poi-scratchpad/src/main/java/org/apache/poi/hslf/usermodel/HSLFTextRun.java:
##########
@@ -440,6 +440,23 @@ public void setFontColor(PaintStyle color) {
         setFontColor(rgb);
     }
 
+
+    @Override
+    public PaintStyle getHighlightColor() {
+        return null;
+    }
+
+    @Override
+    public void setHighlightColor(final Color color) {

Review Comment:
   I agree, but the common rendering in DrawTextParagraph does not have access to the XSLF* classes, so cannot be cast - I chose this as a tradeoff. If you have a better suggestion then Im happy to adjust as needed.



##########
poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java:
##########
@@ -157,6 +160,52 @@ private static void fetchFontColor(CTTextCharacterProperties props, Consumer<Pai
         }
     }
 
+    @Override

Review Comment:
   Done



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] pjfanning commented on a diff in pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #409:
URL: https://github.com/apache/poi/pull/409#discussion_r1057895793


##########
poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java:
##########
@@ -158,6 +162,78 @@ private static void fetchFontColor(CTTextCharacterProperties props, Consumer<Pai
     }
 
 
+    /**
+     * Returns the font highlight (background) color for this text run.
+     * This returns a {@link SolidPaint}, or null if no highlight is set.
+     *
+     * @return The font highlight (background) colour associated with the run, null if no highlight.
+     *
+     * @see org.apache.poi.sl.draw.DrawPaint#getPaint(java.awt.Graphics2D, PaintStyle)
+     * @see SolidPaint#getSolidColor()
+     * @since POI 5.2.4
+     */
+    @Override
+    public PaintStyle getHighlightColor() {
+        XSLFShape shape = getParagraph().getParentShape();
+        final boolean hasPlaceholder = shape.getPlaceholder() != null;
+        return fetchCharacterProperty((props, val) -> fetchHighlightColor(props, val, shape, hasPlaceholder));

Review Comment:
   can we rename the `val` variable to something more informative?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] gffloodg commented on a diff in pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
gffloodg commented on code in PR #409:
URL: https://github.com/apache/poi/pull/409#discussion_r1058339320


##########
poi-ooxml/src/main/java/org/apache/poi/xslf/usermodel/XSLFTextRun.java:
##########
@@ -158,6 +162,78 @@ private static void fetchFontColor(CTTextCharacterProperties props, Consumer<Pai
     }
 
 
+    /**
+     * Returns the font highlight (background) color for this text run.
+     * This returns a {@link SolidPaint}, or null if no highlight is set.
+     *
+     * @return The font highlight (background) colour associated with the run, null if no highlight.
+     *
+     * @see org.apache.poi.sl.draw.DrawPaint#getPaint(java.awt.Graphics2D, PaintStyle)
+     * @see SolidPaint#getSolidColor()
+     * @since POI 5.2.4
+     */
+    @Override
+    public PaintStyle getHighlightColor() {
+        XSLFShape shape = getParagraph().getParentShape();
+        final boolean hasPlaceholder = shape.getPlaceholder() != null;
+        return fetchCharacterProperty((props, val) -> fetchHighlightColor(props, val, shape, hasPlaceholder));
+    }
+
+
+    private static void fetchHighlightColor(CTTextCharacterProperties props, Consumer<PaintStyle> val, XSLFShape shape, boolean hasPlaceholder) {
+        if (props == null) {
+            return;
+        }
+
+        final CTColor col = props.getHighlight();
+        if (col == null) {
+            return;
+        }
+
+        final CTSRgbColor rgbCol = col.getSrgbClr();
+        final byte[] cols = rgbCol.getVal();
+        final SolidPaint paint = DrawPaint.createSolidPaint(new Color(0xFF & cols[0], 0xFF & cols[1], 0xFF & cols[2]));
+        val.accept(paint);
+    }
+
+    /**
+     * Sets the font highlight (background) color for this text run - convenience function
+     *
+     * @param color The highlight (background) color to set.
+     * @since POI 5.2.4
+     */
+    @Override
+    public void setHighlightColor(final Color color) {
+        setHighlightColor(DrawPaint.createSolidPaint(color));
+    }
+
+    /**
+     * Set the highlight (background) color for this text run.
+     *
+     * @param color The highlight (background) color to set.
+     *
+     * @see org.apache.poi.sl.draw.DrawPaint#createSolidPaint(Color)
+     * @since POI 5.2.4
+     */
+    @Override
+    public void setHighlightColor(final PaintStyle color) {
+        if (!(color instanceof SolidPaint)) {
+            LOG.atWarn().log("Currently only SolidPaint is supported!");
+            return;

Review Comment:
   Updated and added tests



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org


[GitHub] [poi] pjfanning commented on a diff in pull request #409: Textruns not honouring highlight

Posted by GitBox <gi...@apache.org>.
pjfanning commented on code in PR #409:
URL: https://github.com/apache/poi/pull/409#discussion_r1056789642


##########
poi/src/main/java/org/apache/poi/sl/usermodel/TextRun.java:
##########
@@ -79,6 +80,37 @@ enum FieldType {
     void setFontColor(PaintStyle color);
 
 
+    /**
+     * Returns the font highlight (background) color for this text run.
+     * This returns a {@link SolidPaint}, or null if no highlight is set.

Review Comment:
   You could create a new interface in this package called `HightlightColorSupport` or something similar. Add these methods there instead. HSLFTextRun will not implement HightlightColorSupport. XSLFTextRun will.
   
   DrawTextParagraph can check if the text run is an instanceof HightlightColorSupport.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@poi.apache.org
For additional commands, e-mail: dev-help@poi.apache.org