code to create chart without reading template file in PPT, Test Example file for DOC and PPT #135

sandeeptiwari32 · 2018-11-28T18:30:12Z

added code to create chart without reading template file in PPT and Test Example file for DOC and PPT

update forked Repo

sandeeptiwari32 · 2018-11-28T18:33:00Z

src/ooxml/java/org/apache/poi/xddf/usermodel/chart/XDDFChart.java

@@ -81,6 +82,27 @@

 @Beta
 public abstract class XDDFChart extends POIXMLDocumentPart implements TextContainer {
+


moved constant variable code from XWPFChart TO XDDFChart

with the moved constants, we can't delete them from XWPFChart as it would cause issues for other users but could you set the XWPFChart values to be be based on the XDDF ones?

public static final int DEFAULT_WIDTH = XDDFChart.DEFAULT_WIDTH;

got it...
merged required code.

merged required code.

asfgit · 2018-11-28T18:33:41Z

Can one of the admins verify this patch?

sandeeptiwari32 · 2018-11-28T18:34:37Z

src/ooxml/java/org/apache/poi/xddf/usermodel/chart/XDDFChart.java

@@ -712,10 +734,29 @@ public CellReference setSheetTitle(String title, int column) {
        XSSFRow row = this.getRow(sheet, 0);
        XSSFCell cell = this.getCell(row, column);
        cell.setCellValue(title);
-        this.updateSheetTable(sheet.getTables().get(0).getCTTable(), title, column);
+


if sheet does't have table object then first create it and then pass newly created table to update sheet table data. so for that purpose created a method GetSheetTable to return table object.

sandeeptiwari32 · 2018-11-28T18:36:30Z

src/ooxml/java/org/apache/poi/xddf/usermodel/chart/XDDFChart.java

        return new CellReference(sheet.getSheetName(), 0, column, true, true);
    }

+    /**


method which check whether sheet have table object, in case table size is 0 then create table and return that table.

sandeeptiwari32 · 2018-11-28T18:39:14Z

src/ooxml/java/org/apache/poi/xddf/usermodel/chart/XDDFChart.java

@@ -729,9 +770,11 @@ public CellReference setSheetTitle(String title, int column) {
    private void updateSheetTable(CTTable ctTable, String title, int index) {
        CTTableColumns tableColumnList = ctTable.getTableColumns();
        CTTableColumn column = null;
-        for( int i = 0; tableColumnList.getCount() < index; i++) {
+        int columnCount  = tableColumnList.getTableColumnList().size()-1;


instead of starting loop form 0 each time start it with table column size. previously each time we are adding new column from 0 inseatd from required index.

sandeeptiwari32 · 2018-11-28T18:41:17Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java

+        return chart;
+    }
+
+    /**


added a method to create template for chart as we have method for XWPFDocument and XSSFWorkbook.

sandeeptiwari32 · 2018-11-28T18:42:44Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java

@@ -416,7 +425,7 @@ public XSLFNotesMaster getNotesMaster() {
     * Return all the charts in the slideshow
     */
    public List<XSLFChart> getCharts() {
-        return _charts;
+        return Collections.unmodifiableList(_charts);


changed it to unmodifiable list as we have unmodifiable list in XWPFDocument and XSSFWorkbook.

great catch!

thank you Alain Sir... :)

sandeeptiwari32 · 2018-11-28T18:43:37Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFChart.java

@@ -90,4 +104,46 @@ protected CTTextBody getTextBody(boolean create) {
            };
        }
    }
+


method to create graphic frame for chart object in XSLFSLIDE

sandeeptiwari32 · 2018-11-28T18:44:09Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFDrawing.java

@@ -106,6 +106,18 @@ public XSLFTable createTable(){
        shape.setAnchor(new Rectangle2D.Double());
        return shape;
    }
+


method to add chart into graphic frame of slide

sandeeptiwari32 · 2018-11-28T18:45:07Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java

@@ -718,5 +720,32 @@ public XSLFPlaceholderDetails getPlaceholderDetails(Placeholder placeholder) {
        final XSLFSimpleShape ph = getPlaceholder(placeholder);
        return (ph == null) ? null : new XSLFPlaceholderDetails(ph);
    }
+
+    /**
+     * this method will add chart into slide


method to add chart into slide with default height width and co-ordinate.

sandeeptiwari32 · 2018-11-28T18:45:32Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java

+        return addChart(chart, rect2D);
+    }
+
+    /**


method to add chart into slide with user defined height width and co-ordinate.

sandeeptiwari32 · 2018-11-28T18:46:04Z

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFChart.java

-     */
-    public static final int DEFAULT_HEIGHT = 500000;
-
+


moved code into XDDFChart class

Like already mentioned in other review, please refrain from deleting public code that other people might already be using in their production code.

Keep the definition of these constants as being = XDDFChart.DEFAULT_WIDTH and = XDDFChart.DEFAULT_HEIGHT

merged required code.

sandeeptiwari32 · 2018-11-28T18:46:37Z

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFDocument.java

@@ -1672,7 +1673,7 @@ public XWPFDocument getXWPFDocument() {
     * @since POI 4.0.0
     */
    public XWPFChart createChart() throws InvalidFormatException, IOException {
-        return createChart(XWPFChart.DEFAULT_WIDTH, XWPFChart.DEFAULT_HEIGHT);
+        return createChart(XDDFChart.DEFAULT_WIDTH, XDDFChart.DEFAULT_HEIGHT);


changed constant class name

sandeeptiwari32 · 2018-11-28T18:47:14Z

src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFSlide.java

@@ -194,4 +195,16 @@ public void testMergeSlides() throws IOException {

        ppt.close();
    }    
+
+    @Test


junit test case for new method in XMLSlideShow "createChart"

sandeeptiwari32

code to create chart without reading template file for PPT

sandeeptiwari32 · 2018-11-28T18:48:29Z

code to create chart without reading template file for PPT

pjfanning · 2018-11-28T19:43:08Z

src/ooxml/java/org/apache/poi/xddf/usermodel/chart/XDDFChart.java

            column = tableColumnList.addNewTableColumn();
            column.setId(i);
+            ++columnCount;
        }


Could you use this?

for (int i = columnCount; i < index; i++) { column = tableColumnList.addNewTableColumn(); column.setId(i); }

my bad.. :P
done required change.

required to change loop condition to 'i' instead of columnCount.

Alain-Bearez

I am so glad to see such a useful contribution complementing my original use case!

Alain-Bearez · 2018-11-29T09:43:53Z

src/examples/src/org/apache/poi/xwpf/usermodel/examples/AutoChartDOCExample.java

+        	Double[] values2 = listSpeakers.toArray(new Double[listSpeakers.size()]);
+
+        	try (XWPFDocument doc = new XWPFDocument()) {
+        		XWPFChart chart = doc.createChart(5000000,5000000);


Please use XDDFChart.DEFAULT_WIDTH and XDDFChart.DEFAULT_HEIGHT instead of these magic numbers.

chnaged hard coded value to constant.

Alain-Bearez · 2018-11-29T09:47:28Z

src/examples/src/org/apache/poi/xslf/usermodel/AutoChartPPTExample.java

+        		XSLFSlide slide = ppt.createSlide();
+        		XSLFChart chart = ppt.createChart();
+        		Rectangle2D rect2D = new java.awt.Rectangle(XDDFChart.DEFAULT_X, XDDFChart.DEFAULT_Y, 
+        				5000000, 5000000);


Please use XDDFChart.DEFAULT_WIDTH and XDDFChart.DEFAULT_HEIGHT instead of these magic numbers.

chnaged hard coded value to constant

Alain-Bearez · 2018-11-29T09:52:29Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java

+    /**
+     * This method is used to create template for chart XML.
+     * @return Xslf chart object 
+     */


add the @since 4.0.2 as we might not be able to ship it with currently reviewed 4.0.1 to be released after the week-end

Added Tag POI 4.0.2

Alain-Bearez · 2018-11-29T09:53:05Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java

        chart.setChartIndex(chartIdx);
        _charts.add(chart);
        return chart;
    }
-
+    


no trailing whitespaces, please

removed space.

Alain-Bearez · 2018-11-29T09:53:26Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XMLSlideShow.java

@@ -416,7 +425,7 @@ public XSLFNotesMaster getNotesMaster() {
     * Return all the charts in the slideshow
     */
    public List<XSLFChart> getCharts() {
-        return _charts;
+        return Collections.unmodifiableList(_charts);


great catch!

Alain-Bearez · 2018-11-29T09:57:00Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFChart.java

+     * @param anchor size and location of chart
+     * @return graphic frame object
+     */
+    static CTGraphicalObjectFrame prototype(int shapeId, String rID, Rectangle2D anchor){


Does this really need to be of package visibility? Wouldn't private be OK?

we are using this method from XSLF drawing as we are using for other shap controls. so need protected or package visibility.

Alain-Bearez · 2018-11-29T09:58:11Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFDrawing.java

+     *
+     * @param rID relation id of chart
+     * @param rect2D Chart Bounding values
+     */


add the @since 4.0.2 as we might not be able to ship it with currently reviewed 4.0.1 to be released after the week-end

Added Tag POI 4.0.2

Alain-Bearez · 2018-11-29T09:58:32Z

src/ooxml/java/org/apache/poi/xslf/usermodel/XSLFSheet.java

+     * with default height, width, x and y
+     * @param chart xslf chart object
+     * @return xslf chart object
+     */


add the @since 4.0.2 as we might not be able to ship it with currently reviewed 4.0.1 to be released after the week-end

Added Tag POI 4.0.2

Alain-Bearez · 2018-11-29T10:03:00Z

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFChart.java

-     */
-    public static final int DEFAULT_HEIGHT = 500000;
-
+


Like already mentioned in other review, please refrain from deleting public code that other people might already be using in their production code.

Keep the definition of these constants as being = XDDFChart.DEFAULT_WIDTH and = XDDFChart.DEFAULT_HEIGHT

Alain-Bearez · 2018-11-29T10:04:57Z

src/ooxml/testcases/org/apache/poi/xslf/usermodel/TestXSLFSlide.java

+        XSLFChart chart = ppt.createChart();
+        slide.addChart(chart);
+
+        assertNotNull(chart); 


I would have checked that the returned chart from slide.addChart is the one you expect to be, i.e. the original chart passed as argument.

changed method slide.addChart return type. no need to return XSLFchart object. we are using XSLFChart object to add relationship into slide.

instead of hard coded value changed it to constant.

instead of hard coded value changed it to constant

merged required changes

added since POI 4.0.2 tag

Added POI 4.0.2 Tag

removed trailing space

changed visibility CHART_URI constant and Added POI 4.0.2 Tag

Added POI 4.0.2 Tag

updated test cases.

changed method, no required to return XLSFChart object.

sandeeptiwari32 · 2018-11-29T17:35:28Z

added required changes.

updated method comment

Alain-Bearez

3 more details

Alain-Bearez · 2018-11-30T07:58:12Z

src/ooxml/java/org/apache/poi/xddf/usermodel/chart/XDDFChart.java

+     * in case table size zero then create new table and add table columns element
+     * @param sheet
+     * @return table object
+     * @since POI 4.0.2


There is no need for @since tags on private methods.

Alain-Bearez · 2018-11-30T08:00:41Z

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFChart.java

@@ -36,16 +36,16 @@ Licensed to the Apache Software Foundation (ASF) under one or more
 */
 @Beta
 public class XWPFChart extends XDDFChart {
-    /**
+   /**


why the change in indentation here?

might be appeared because i edited it github web portal.
managed indentation.

Alain-Bearez · 2018-11-30T08:01:14Z

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFChart.java

-    public static final int DEFAULT_HEIGHT = 500000;
-
+    public static final int DEFAULT_HEIGHT = XDDFChart.DEFAULT_HEIGHT;
+   


no trailing whitespace, please!

might be appeared because i edited it github web portal.
removed space.

changed indentation

changed indentation.

removed since tag

sandeeptiwari32 · 2018-12-01T05:07:14Z

src/ooxml/java/org/apache/poi/xddf/usermodel/chart/XDDFChart.java

+     * this method will check whether sheet have table
+     * in case table size zero then create new table and add table columns element
+     * @param sheet
+     * @return table object


removed since tag

sandeeptiwari32 · 2018-12-01T05:08:19Z

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFChart.java

@@ -39,12 +39,12 @@ Licensed to the Apache Software Foundation (ASF) under one or more
    /**


might be appeared because i edited it github web portal.
managed indentation.

sandeeptiwari32 · 2018-12-01T05:09:01Z

src/ooxml/java/org/apache/poi/xwpf/usermodel/XWPFChart.java


    /**
     * default height of chart in emu
     */
-    public static final int DEFAULT_HEIGHT = 500000;
+    public static final int DEFAULT_HEIGHT = XDDFChart.DEFAULT_HEIGHT;



might be appeared because i edited it github web portal.
removed space.

sandeeptiwari32 · 2018-12-01T05:10:13Z

updated required changes.

pjfanning · 2018-12-03T08:09:46Z

@Alain-Bearez POI 4.0.1 is released. Do you think it is ok to merge this now?

sandeeptiwari32 · 2018-12-07T16:23:53Z

@Alain-Bearez is it ok to merge current changes, or still need to change something.

Alain-Bearez · 2018-12-08T02:29:59Z

I am still fixing some formatting minor issues before merging.

Alain-Bearez · 2018-12-08T02:47:56Z

merged by r1848432

Alain-Bearez · 2018-12-08T03:04:51Z

@pjfanning how could we make sure all the required new CT* classes on
https://github.com/apache/poi/pull/135/files#diff-e7bf6663c76af8916ed221806ed98fa0
are present in the ooxml-lite?

Only then you could close this PR.

pjfanning · 2018-12-08T08:26:30Z

@sandeeptiwari32 can you close the PR? I ran a quick check and it looks like we have all the XMLBeans generated classes that we need in poi-ooxml-schemas jar. That jar is a subset of the full ooxml-schemas.jar and the classes in subset are derived from the classes that are used in our unit tests.

sandeeptiwari32 · 2018-12-08T14:11:06Z

@pjfanning sure.

sandeeptiwari32 · 2018-12-08T14:11:41Z

close pull request.

18876293672 · 2019-09-24T07:36:40Z

.......................................................................................

sandeeptiwari32 added 2 commits November 28, 2018 22:17

Merge pull request #1 from apache/trunk

a43a529

update forked Repo

code to create chart in ppt without reading template file

270cba7

sandeeptiwari32 commented Nov 28, 2018

View reviewed changes

pjfanning reviewed Nov 28, 2018

View reviewed changes

changed loop condition

68382e5

required to change loop condition to 'i' instead of columnCount.

Alain-Bearez suggested changes Nov 29, 2018

View reviewed changes

sandeeptiwari32 added 9 commits November 29, 2018 20:55

instead of hard coded value changed it to constant

a3d28fe

instead of hard coded value changed it to constant.

instead of hard coded value changed it to constant

e01f35e

instead of hard coded value changed it to constant

merged required changes

bd7c3c5

merged required changes

added since POI 4.0.2 tag

f058e97

added since POI 4.0.2 tag

Added POI 4.0.2 Tag

525581d

Added POI 4.0.2 Tag

removed trailing space

176255e

removed trailing space

changed visibility CHART_URI Added POI 4.0.2 Tag

6c218a9

changed visibility CHART_URI constant and Added POI 4.0.2 Tag

Added POI 4.0.2 Tag

c020bdf

Added POI 4.0.2 Tag

Added POI 4.0.2 Tag

cd04b4f

Added POI 4.0.2 Tag

sandeeptiwari32 added 2 commits November 29, 2018 22:46

updated test cases.

53228de

updated test cases.

changed method, no required to return.

6d488ea

changed method, no required to return XLSFChart object.

updated method comment

8b5c28a

updated method comment

Alain-Bearez reviewed Nov 30, 2018

View reviewed changes

sandeeptiwari32 added 3 commits December 1, 2018 10:19

changed indentation

761f3ed

changed indentation

changed indentation.

a45c87b

changed indentation.

removed since tag

2929f97

removed since tag

sandeeptiwari32 commented Dec 1, 2018

View reviewed changes

Alain-Bearez approved these changes Dec 8, 2018

View reviewed changes

sandeeptiwari32 closed this Dec 8, 2018

		@@ -81,6 +82,27 @@

		@Beta
		public abstract class XDDFChart extends POIXMLDocumentPart implements TextContainer {

@@ @@ -90,4 +104,46 @@ protected CTTextBody getTextBody(boolean create) { @@
                           };
                       }
                   }

		public static final int DEFAULT_HEIGHT = 500000;

		public static final int DEFAULT_HEIGHT = XDDFChart.DEFAULT_HEIGHT;

		@@ -39,12 +39,12 @@ Licensed to the Apache Software Foundation (ASF) under one or more
		/**

code to create chart without reading template file in PPT, Test Example file for DOC and PPT #135

code to create chart without reading template file in PPT, Test Example file for DOC and PPT #135

Conversation

sandeeptiwari32 commented Nov 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asfgit commented Nov 28, 2018

sandeeptiwari32 Nov 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandeeptiwari32 left a comment

Choose a reason for hiding this comment

sandeeptiwari32 commented Nov 28, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Alain-Bearez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandeeptiwari32 commented Nov 29, 2018

Alain-Bearez left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sandeeptiwari32 commented Dec 1, 2018

pjfanning commented Dec 3, 2018

sandeeptiwari32 commented Dec 7, 2018 • edited Loading

Alain-Bearez commented Dec 8, 2018

Alain-Bearez commented Dec 8, 2018

Alain-Bearez commented Dec 8, 2018

pjfanning commented Dec 8, 2018

sandeeptiwari32 commented Dec 8, 2018

sandeeptiwari32 commented Dec 8, 2018

18876293672 commented Sep 24, 2019

sandeeptiwari32 Nov 28, 2018 •

edited

Loading

sandeeptiwari32 commented Dec 7, 2018 •

edited

Loading