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#135sandeeptiwari32 wants to merge 18 commits intoapache:trunkfrom sandeeptiwari32:trunk
Conversation
|
|
||
| @Beta | ||
| public abstract class XDDFChart extends POIXMLDocumentPart implements TextContainer { | ||
|
|
There was a problem hiding this comment.
moved constant variable code from XWPFChart TO XDDFChart
There was a problem hiding this comment.
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;
There was a problem hiding this comment.
got it...
merged required code.
|
Can one of the admins verify this patch? |
| XSSFCell cell = this.getCell(row, column); | ||
| cell.setCellValue(title); | ||
| this.updateSheetTable(sheet.getTables().get(0).getCTTable(), title, column); | ||
|
|
There was a problem hiding this comment.
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); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
method which check whether sheet have table object, in case table size is 0 then create table and return that table.
| CTTableColumns tableColumnList = ctTable.getTableColumns(); | ||
| CTTableColumn column = null; | ||
| for( int i = 0; tableColumnList.getCount() < index; i++) { | ||
| int columnCount = tableColumnList.getTableColumnList().size()-1; |
There was a problem hiding this comment.
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; | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
added a method to create template for chart as we have method for XWPFDocument and XSSFWorkbook.
| */ | ||
| public List<XSLFChart> getCharts() { | ||
| return _charts; | ||
| return Collections.unmodifiableList(_charts); |
There was a problem hiding this comment.
changed it to unmodifiable list as we have unmodifiable list in XWPFDocument and XSSFWorkbook.
There was a problem hiding this comment.
thank you Alain Sir... :)
| }; | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
method to create graphic frame for chart object in XSLFSLIDE
| shape.setAnchor(new Rectangle2D.Double()); | ||
| return shape; | ||
| } | ||
|
|
There was a problem hiding this comment.
method to add chart into graphic frame of slide
| } | ||
|
|
||
| /** | ||
| * this method will add chart into slide |
There was a problem hiding this comment.
method to add chart into slide with default height width and co-ordinate.
| return addChart(chart, rect2D); | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
method to add chart into slide with user defined height width and co-ordinate.
| */ | ||
| public static final int DEFAULT_HEIGHT = 500000; | ||
|
|
||
|
|
There was a problem hiding this comment.
moved code into XDDFChart class
There was a problem hiding this comment.
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
| */ | ||
| public XWPFChart createChart() throws InvalidFormatException, IOException { | ||
| return createChart(XWPFChart.DEFAULT_WIDTH, XWPFChart.DEFAULT_HEIGHT); | ||
| return createChart(XDDFChart.DEFAULT_WIDTH, XDDFChart.DEFAULT_HEIGHT); |
There was a problem hiding this comment.
changed constant class name
| ppt.close(); | ||
| } | ||
|
|
||
| @Test |
There was a problem hiding this comment.
junit test case for new method in XMLSlideShow "createChart"
sandeeptiwari32
left a comment
There was a problem hiding this comment.
code to create chart without reading template file for PPT
|
code to create chart without reading template file for PPT |
| column = tableColumnList.addNewTableColumn(); | ||
| column.setId(i); | ||
| ++columnCount; | ||
| } |
There was a problem hiding this comment.
Could you use this?
for (int i = columnCount; i < index; i++) {
column = tableColumnList.addNewTableColumn();
column.setId(i);
}
There was a problem hiding this comment.
my bad.. :P
done required change.
required to change loop condition to 'i' instead of columnCount.
Alain-Bearez
left a comment
There was a problem hiding this 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); |
There was a problem hiding this comment.
Please use XDDFChart.DEFAULT_WIDTH and XDDFChart.DEFAULT_HEIGHT instead of these magic numbers.
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
Please use XDDFChart.DEFAULT_WIDTH and XDDFChart.DEFAULT_HEIGHT instead of these magic numbers.
There was a problem hiding this comment.
chnaged hard coded value to constant
| /** | ||
| * This method is used to create template for chart XML. | ||
| * @return Xslf chart object | ||
| */ |
There was a problem hiding this comment.
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
| return chart; | ||
| } | ||
|
|
||
There was a problem hiding this comment.
no trailing whitespaces, please
| */ | ||
| public List<XSLFChart> getCharts() { | ||
| return _charts; | ||
| return Collections.unmodifiableList(_charts); |
| * @param anchor size and location of chart | ||
| * @return graphic frame object | ||
| */ | ||
| static CTGraphicalObjectFrame prototype(int shapeId, String rID, Rectangle2D anchor){ |
There was a problem hiding this comment.
Does this really need to be of package visibility? Wouldn't private be OK?
There was a problem hiding this comment.
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 | ||
| */ |
There was a problem hiding this comment.
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
| * with default height, width, x and y | ||
| * @param chart xslf chart object | ||
| * @return xslf chart object | ||
| */ |
There was a problem hiding this comment.
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
| */ | ||
| public static final int DEFAULT_HEIGHT = 500000; | ||
|
|
||
|
|
There was a problem hiding this comment.
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); |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
Added POI 4.0.2 Tag
updated test cases.
changed method, no required to return XLSFChart object.
|
added required changes. |
updated method comment
| * 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 was a problem hiding this comment.
There is no need for @since tags on private methods.
| @Beta | ||
| public class XWPFChart extends XDDFChart { | ||
| /** | ||
| /** |
There was a problem hiding this comment.
why the change in indentation here?
There was a problem hiding this comment.
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; | ||
There was a problem hiding this comment.
no trailing whitespace, please!
There was a problem hiding this comment.
might be appeared because i edited it github web portal.
removed space.
changed indentation
changed indentation.
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 |
| @@ -39,12 +39,12 @@ public class XWPFChart extends XDDFChart { | |||
| /** | |||
There was a problem hiding this comment.
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; | ||
|
|
There was a problem hiding this comment.
might be appeared because i edited it github web portal.
removed space.
|
updated required changes. |
|
@Alain-Bearez POI 4.0.1 is released. Do you think it is ok to merge this now? |
|
@Alain-Bearez is it ok to merge current changes, or still need to change something. |
|
I am still fixing some formatting minor issues before merging. |
|
merged by r1848432 |
|
@pjfanning how could we make sure all the required new Only then you could close this PR. |
|
@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. |
|
@pjfanning sure. |
|
close pull request. |
|
....................................................................................... |
added code to create chart without reading template file in PPT and Test Example file for DOC and PPT