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

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

Closed
wants to merge 18 commits into from

Conversation

@sandeeptiwari32
Copy link

commented Nov 28, 2018

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

Merge pull request #1 from apache/trunk
update forked Repo
@@ -81,6 +82,27 @@

@Beta
public abstract class XDDFChart extends POIXMLDocumentPart implements TextContainer {

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

moved constant variable code from XWPFChart TO XDDFChart

This comment has been minimized.

Copy link
@pjfanning

pjfanning Nov 29, 2018

Contributor

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; 

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

got it...
merged required code.

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

merged required code.

@asfgit

This comment has been minimized.

Copy link

commented Nov 28, 2018

Can one of the admins verify this patch?

@@ -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);

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

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.

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

/**

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

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

@@ -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;

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

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.

return chart;
}

/**

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

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

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

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

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

great catch!

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

thank you Alain Sir... :)

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

method to create graphic frame for chart object in XSLFSLIDE

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

method to add chart into graphic frame of slide

@@ -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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

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

return addChart(chart, rect2D);
}

/**

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

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

*/
public static final int DEFAULT_HEIGHT = 500000;


This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

moved code into XDDFChart class

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

merged required code.

@@ -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);

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

changed constant class name

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

ppt.close();
}

@Test

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 28, 2018

Author

junit test case for new method in XMLSlideShow "createChart"

@sandeeptiwari32
Copy link
Author

left a comment

code to create chart without reading template file for PPT

@sandeeptiwari32

This comment has been minimized.

Copy link
Author

commented Nov 28, 2018

code to create chart without reading template file for PPT

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

This comment has been minimized.

Copy link
@pjfanning

pjfanning Nov 28, 2018

Contributor

Could you use this?

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

my bad.. :P
done required change.

changed loop condition
required to change loop condition to 'i' instead of  columnCount.
@Alain-Bearez
Copy link
Contributor

left a comment

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

Double[] values2 = listSpeakers.toArray(new Double[listSpeakers.size()]);

try (XWPFDocument doc = new XWPFDocument()) {
XWPFChart chart = doc.createChart(5000000,5000000);

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

chnaged hard coded value to constant.

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

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

chnaged hard coded value to constant

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

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

Added Tag POI 4.0.2

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

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

no trailing whitespaces, please

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

removed space.

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

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

great catch!

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

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

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

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

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

Added Tag POI 4.0.2

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

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

Added Tag POI 4.0.2

*/
public static final int DEFAULT_HEIGHT = 500000;


This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

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

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

assertNotNull(chart);

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 29, 2018

Contributor

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.

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Nov 29, 2018

Author

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.
instead of hard coded value changed it to constant
instead of hard coded value changed it to constant
merged required changes
merged required changes
added since POI 4.0.2 tag
added since POI 4.0.2 tag
Added POI 4.0.2 Tag
Added POI 4.0.2 Tag
removed trailing space
removed trailing space
changed visibility CHART_URI Added POI 4.0.2 Tag
changed visibility CHART_URI constant and Added POI 4.0.2 Tag
Added POI 4.0.2 Tag
Added POI 4.0.2 Tag
Added POI 4.0.2 Tag
Added POI 4.0.2 Tag
updated test cases.
updated test cases.
changed method, no required to return.
changed method, no required to return XLSFChart object.
@sandeeptiwari32

This comment has been minimized.

Copy link
Author

commented Nov 29, 2018

added required changes.

updated method comment
updated method comment
@Alain-Bearez
Copy link
Contributor

left a comment

3 more details

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

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 30, 2018

Contributor

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

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

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 30, 2018

Contributor

why the change in indentation here?

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Dec 1, 2018

Author

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

public static final int DEFAULT_HEIGHT = 500000;

public static final int DEFAULT_HEIGHT = XDDFChart.DEFAULT_HEIGHT;

This comment has been minimized.

Copy link
@Alain-Bearez

Alain-Bearez Nov 30, 2018

Contributor

no trailing whitespace, please!

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Dec 1, 2018

Author

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

changed indentation
changed indentation
changed indentation.
changed indentation.
removed since tag
removed since tag
* 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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Dec 1, 2018

Author

removed since tag

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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Dec 1, 2018

Author

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


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

This comment has been minimized.

Copy link
@sandeeptiwari32

sandeeptiwari32 Dec 1, 2018

Author

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

@sandeeptiwari32

This comment has been minimized.

Copy link
Author

commented Dec 1, 2018

updated required changes.

@pjfanning

This comment has been minimized.

Copy link
Contributor

commented Dec 3, 2018

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

@sandeeptiwari32

This comment has been minimized.

Copy link
Author

commented Dec 7, 2018

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

@Alain-Bearez

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2018

I am still fixing some formatting minor issues before merging.

@Alain-Bearez

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2018

merged by r1848432

@Alain-Bearez

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2018

@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

This comment has been minimized.

Copy link
Contributor

commented Dec 8, 2018

@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

This comment has been minimized.

Copy link
Author

commented Dec 8, 2018

@pjfanning sure.

@sandeeptiwari32

This comment has been minimized.

Copy link
Author

commented Dec 8, 2018

close pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants
You can’t perform that action at this time.