Issue 91578

Summary: Display of hidden cells
Product: General Reporter: IngridvdM
Component: chartAssignee: kla <thomas.klarhoefer>
Status: CLOSED FIXED QA Contact: issues@graphics <issues>
Severity: Trivial    
Priority: P3 CC: amy2008, issues, kyoshida, yoshimit
Version: 3.3.0 or older (OOo)Keywords: ms_interoperability, usability
Target Milestone: ---   
Hardware: All   
OS: All   
URL: https://bugzilla.novell.com/show_bug.cgi?id=408424
Issue Type: FEATURE Latest Confirmation in: ---
Developer Difficulty: ---
Attachments:
Description Flags
example scenario
none
new tab page for plotting options
none
options tab page for pie chart
none
screenshot - different wording
none
example chart with data from a writer table
none
chart without a range for categories
none
example with wrong space and wrong legend entry
none
double click the chart to see the problem with missing values
none
example for status bar text of selected trenline
none
TCS Display_of_hidden_cells.htm none

Description IngridvdM 2008-07-11 15:03:30 UTC
When Charts are created from Calc ranges with partly hidden cells the user
should be able to choose whether the hidden values should or should not be
displayed in the chart.
Comment 1 IngridvdM 2008-07-11 15:15:35 UTC
Created attachment 55072 [details]
example scenario
Comment 2 IngridvdM 2008-07-11 15:20:17 UTC
It seems that the behavior for this scenario has changed several times in the
past and always there were users that were not happy with the current behavior
so it has become necessary to offer an option to choose.
Comment 3 IngridvdM 2008-07-11 15:23:52 UTC
*** Issue 80626 has been marked as a duplicate of this issue. ***
Comment 4 IngridvdM 2008-07-11 15:30:57 UTC
*** Issue 52800 has been marked as a duplicate of this issue. ***
Comment 5 IngridvdM 2008-07-11 15:56:29 UTC
In the user interface I could imagine a simple check box 'Display values from
hidden cells' or similar. Two possible places for the check box come to my mind:
1) On the 'Data Range' tab page - just below 'First column as label' maybe.
2) 'Options' tab page within the series properties dialog.
Issue 65549 in CWS chart22 does introduce the vaguely related setting 'Plot
missing values' exactly on the series options page.
So are there any user opinions about where to place the check box?
Comment 6 IngridvdM 2008-07-11 16:20:00 UTC
@kohei, yes for the file format we would need only a simple boolean. I think it
would be elegant to define the new attribute in parallel to the somehow related
attribute 'chart:treat-empty-cells' - having it in the style at the plot area.
Here is a first try for the proposal:
----
The chart:display-hidden-cells attribute specifies whether values from hidden
cells are displayed within the chart or not.
This attribute can be used within styles that are applied to a <chart:plot-area>
element.
The chart:treat-empty-cells attribute may be used with the following element:
<style:chart-properties> 

<define name="style-chart-properties-attlist" combine="interleave">
	<optional>
		<attribute name="chart:display-hidden-cells">
			<ref name="boolean"/>
		</attribute>
	</optional>
</define>
----
I am not sure about a default. In the past the behavior has changed again and again.
Kohei, as you are on the TC list, would you mind to propose this new attribute?
With luck we can have it for ODF1.2 maybe. That would be cool.
Comment 7 IngridvdM 2008-07-11 16:35:31 UTC
Oh there is one wrong  'treat-empty-cells' in the proposed proposal text. That
should be replaced with 'display-hidden-cells', sorry.

Comment 8 kyoshida 2008-07-11 16:46:53 UTC
Yeah I can work on proposing that to the TC.  It should be fairly simple since
it's just one boolean, but you never know....

I'll keep you updated with the progress.
Comment 9 IngridvdM 2008-07-11 17:19:25 UTC
@kohei, cool thanks! Some hints for the implementation:
There is already a property that is very similar implemented to what we need
now. It is the check box 'show bars side by side' on the options page in the
series properties dialog for bar and column charts. So it can serve as example.
In the UNO API we need a new property at the diagram maybe 'DisplayHiddenCells'.
The stable ::com::sun::star::chart API is implemented in
chart2/source/controller/chartapiwrapper. Add the new property to the
DiagramWrapper class (good example is 'GroupBarsPerAxis'). The inner chart model
uses not the old stable API but a new ::com::sun::star::chart2 API instead.
There we need to add the same new property at the 'inner' diagram class ( look
at chart2/source/model/main/Diagram.cxx).
In xmloff we need a new token 'display-hidden-cells'. And the new token must be
mapped to the new UNO property. Grep for XML_GROUP_BARS_PER_AXIS in xmloff for
an example. Thus the new property will be written from and to the file.
Now the new property must be transported to the view for display purposes.
Search for 'GroupBarsPerAxis' within chart2/source/view/ChartView.cxx to have an
example. The view does set the property from the diagram model to a view class
'VDataSeries'. We will need a new method SetDisplayHiddenCells(). VDataSeries is
the container for the differenet VDataSequence where we finally need the
property (you found the boolean bDisplayHiddenCells there already  :-) ).
Finally the dialog related things are implemented in chart2/
source/controller/dialogs and
chart2/ source/controller/itemsetwrapper. All the ItemConverters within
itemsetwrapper serve the purpose to transport the UNO API properties from the
model object into SfxItemSets which is a typical input format for several
dialogs and tab pages. And vice versa of course.
I hope the hints are useful for you. Have fun!
Comment 10 kyoshida 2008-09-29 17:56:33 UTC
Ok.  The ODF TC seems to be in favor of the change we have proposed.[1]  There
are some minor suggestions for change, but none are blockers.

[1] http://wiki.oasis-open.org/office/New_Chart_Option_on_Hidden_Cells
Comment 11 IngridvdM 2008-10-01 10:13:37 UTC
*** Issue 28932 has been marked as a duplicate of this issue. ***
Comment 12 kyoshida 2008-10-01 22:42:46 UTC
started.
Comment 13 kyoshida 2008-10-07 15:19:49 UTC
I've pretty much finished all tasks except for the ODF import/export & check box
in the UI.  I'll work on those once we get approval on the ODF spec change from
the TC.
Comment 14 kyoshida 2008-10-07 16:07:11 UTC
As for the location of the check box (sorry I didn't see that you asked for my
opinion earlier), I would like the check box to stay close to the check box for
the "plot missing values" option.  These two options IMO should be placed
side-by-side if possible.
Comment 15 IngridvdM 2008-10-07 16:32:12 UTC
Yes, that sounds like a good idea. The affected tab page class is
SchOptionTabPage in files tp_SeriesToAxis.src -.cxx -hxx. The "plot missing
values" option was introduced there in the 3.1 code line starting with dev300m29.
Comment 16 kyoshida 2008-10-31 14:16:20 UTC
BTW the core work is already done & ODF import/export completed.  I will start
the UI work once I get started on upstreaming the code (since the 3.0 codeline
that I currently work on in ooo-build doesn't have the treat empty cells addition).
Comment 17 kyoshida 2008-11-03 20:58:46 UTC
just tried to create a cws for this.  But 'cws create DEV300 charthiddencells'
created a new svn branch but failed to connect to the EIS.  I have manually
created an entry for that branch via EIS web interface, but it's still incomplete.

So, at the moment I have an out-of-sync cws branch that needs to be repaired by
someone.... :-(
Comment 18 kyoshida 2008-11-04 13:42:39 UTC
I've filed Issue 95828 for the cws create problem.
Comment 19 kyoshida 2008-11-26 06:06:15 UTC
Created attachment 58306 [details]
new tab page for plotting options
Comment 20 kyoshida 2008-11-26 06:08:55 UTC
@iha: I've re-designed the Options tab page and attached the new layout.  What
do you think of this layout?  I had to rename the fixed line text from 'Plot
missing values' to 'Plot options' so that I could squeeze the
include-hidden-cells check box there.  I've gone through a couple of different
layouts, but I like this one best.

Let me know what you think.
Comment 21 kyoshida 2008-11-26 07:13:00 UTC
Created attachment 58307 [details]
options tab page for pie chart
Comment 22 IngridvdM 2008-11-26 14:26:01 UTC
@kohei: Thanks! I think the layout could do. Maybe I would choose slightly
different strings: 'Plot missing values' instead of 'Missing values'  and
'Plot values from hidden cells' instead of 'Include hidden cells', but I am not
a native speaker. I'll attach an according screen shot.
Here is the link to the specification from the 'Missing value' feature:
http://specs.openoffice.org/chart/MissingValues.odt
I think it would be suitable to simply extend this specification to include the
new check box. The spec needs to be changed anyhow now. Have a great day!
Comment 23 IngridvdM 2008-11-26 14:27:32 UTC
Created attachment 58323 [details]
screenshot - different wording
Comment 24 kyoshida 2008-12-01 16:56:53 UTC
@iha: thanks!  I think your wording sounds better.  I'll make those changes &
the spec (hopefully I still remember how to access it...)

P.S. I just got back from our long holidays.
Comment 25 IngridvdM 2008-12-01 17:05:12 UTC
@kohei, the spec is located in module specs/www/chart/MissingValues.odt. It
should be  accessible via cvs still.
P.S.: I hope you had nice holidays! I will start my longer vacation the day
after tomorrow :-)
Comment 26 kyoshida 2008-12-01 19:06:41 UTC
Ok.  Just updated the spec.

@iha: do you think I can assign tk as the QA rep for koheichart01?
Comment 27 kyoshida 2008-12-01 21:09:51 UTC
This is now implemented in koheichart01 cws.
Comment 28 IngridvdM 2008-12-02 14:41:33 UTC
@kohei,
> do you think I can assign tk as the QA rep for koheichart01?
Do you mean Thomas Klarhöfer (kla)? Yes, you can. But he is already completely
busy for 3.1 feature freeze. So if target 3.2 is ok for you then set Thomas as
QA rep!
Comment 29 niklas.nebel 2008-12-04 10:18:43 UTC
Why are formulas set dirty when the chart starts listening (markRangeDirty)?
That looks like a strange hack.
Comment 30 kyoshida 2008-12-04 12:54:58 UTC
@nn: that's not a hack.  Under some circumstances value changes do not trigger
formula broadcasters to send out value changed broadcast to their listeners,
especially when the source ranges are outside the visible area and consist of
formula cells.  Marking them dirty is the only way to ensure that the broadcast
occurs on value change.
Comment 31 niklas.nebel 2008-12-04 19:26:45 UTC
What are these circumstances? If formula broadcasts don't work, other areas
beside charts are affected, and that should be fixed. If it's just about the
charts, then the fix should be limited to charts, and not mess around with
formulas. This just doesn't seem right.
Comment 32 kyoshida 2008-12-04 19:39:08 UTC
Setting formulas dirty is hardly "messing around with formulas" since no tokens
are modified.

Here are the circumstances.  When source ranges are formula cells outside the
visible area, that listens on value cells whose values get changed by the API. 
When the values of the listened cells change via API, those formula cells are
not re-calculated until they become visible, which causes the chart to not get
updated.  Marking them dirty once when the chart gets activated solves this issue.
Comment 33 kyoshida 2008-12-04 19:50:55 UTC
BTW delaying re-calculation until they become visible is "correct" in its own
right.  That only becomes problem when an object such as chart depends on their
values being up-to-date even when they are not visible.  So, to me marking them
dirty to force re-calculation on the next value change upon chart creation seems
logical to me.
Comment 34 niklas.nebel 2008-12-08 18:34:43 UTC
Formula cells do of course re-broadcast changes even if they are not visible
(there might be visible cells depending on them). A formula cell does not
propagate the broadcast if it was already dirty, on the assumption that if the
previous value wasn't used, all formulas that depend on the value are also still
dirty from the previous broadcast.

If you run into this because the cell values aren't read, what you need is the
opposite of setting them dirty. Calculation (and with it, clearing the dirty
flag) can be forced by reading the result value.

ScFormulaCell::SetDirty does more than setting the flag, but you're relying on
quite a chain of effects here, which seems too fragile.
Comment 35 kyoshida 2008-12-18 14:59:10 UTC
Ok.  I've removed the controversial markRangeDirty method for now.  That means
that my test case that the method fixes will be back to a broken state, but at
least it won't mess up the existing formula handling because of this code change.

I will look into that formula issue separately.  Since it's hard to reproduce it
in a small test case document, and the only reproducing document I have is very
large & impossible to debug with it, I'd like to tackle is as a separate task.
Comment 36 kyoshida 2008-12-18 21:49:56 UTC
Jon (Pryor) did a QA on this, and found a bug with the tool tips.  The tool tips
in a chart act as if all data points are shown.  I need to look into this to fix it.
Comment 37 kyoshida 2008-12-24 15:12:08 UTC
Just fixed the tooltip issue.  The tooltips inside chart objects now display
proper data point values even when some of them are hidden.

This is once again fixed in koheichart01 cws.
Comment 38 kyoshida 2009-01-05 20:42:27 UTC
Reopening it as we uncovered another regression.  It is actually filed in Issue
97738, so the detail of the regression is described there.
Comment 39 kyoshida 2009-01-05 20:43:58 UTC
In short, the x-axis (category axis) fails to display its values if they are
numeric, which also includes dates.

I'm looking in this here:
https://bugzilla.novell.com/show_bug.cgi?id=463199
Comment 40 kyoshida 2009-01-05 22:52:37 UTC
just fixed the x-axis issue.
Comment 41 kyoshida 2009-01-14 18:04:48 UTC
Re-opening this again.  The tooltip issue is not entirely fixed.  The value is
displayed correctly, but the category name isn't.

I need to fix this.
Comment 42 kyoshida 2009-01-14 18:50:16 UTC
Luckily it was not so hard to fix.  Fixed again.
Comment 43 IngridvdM 2009-02-13 14:08:30 UTC
@kohei, I found that the current version does break charts with categories from
writer tables. The properties 'IsHidden' and 'HiddenValues' are optional and not
supported by writer so far. Load the attached document ChartFromWriterTable.odt,
double click the chart and you will see how the diagram vanishes, what it should
not.
Comment 44 IngridvdM 2009-02-13 14:09:26 UTC
Created attachment 60139 [details]
example chart with data from a writer table
Comment 45 IngridvdM 2009-02-13 14:09:59 UTC
Another problem is with charts that do not have a range for categories.
For those charts there should be automatically generated some strings, but they
are missing now. Load the attached document ChartWithoutCategoryRange.ods and
double click on the chart. You will see how the categories vanish what they
should not.
Comment 46 IngridvdM 2009-02-13 14:10:40 UTC
Created attachment 60140 [details]
chart without a range for categories
Comment 47 IngridvdM 2009-02-13 14:11:29 UTC
@kohei, I have fixed both problems already locally and could commit the changes
directly to the CWS. Would that be OK for you?
Comment 48 kyoshida 2009-02-13 14:21:59 UTC
@iha: yup, that's fine.  Please commit away. :-)

BTW, both problems sound familiar.  Those were something we had encountered in
ooo-build and I thought I had already fixed that in koheichart01 as well... :-/

Anyway, thanks for the fix.
Comment 49 kyoshida 2009-02-13 14:37:13 UTC
@iha: BTW I'm going to resync that cws once koheiformula02 gets integrated. 
koheiformula02 modified chart2uno.[hc]xx quite a lot, so I have no doubt that
its integration will clash with the change in koheichart01.
Comment 50 IngridvdM 2009-02-13 14:43:48 UTC
I have commited fixes for both above described problems to CWS koheichart01.
@kohei, are there more bugs in CWS koheichart01 that are already known? I am
asking because I am willing to have a deeper look into the chart changes on this
CWS and have the time to do so at the moment. But if it is an outdated version
I'd better wait until you have brought the known fixes into it maybe. What do
you think does it make sense to look at further at the moment? I have seen for
example that the legend entries for 'hidden' series do not hide.
Comment 51 kyoshida 2009-02-13 14:57:33 UTC
@iha: no, it's not outdated.  I have made every effort to sync this cws with
ooo-build's.  Those fixes that I mentioned were already in koheichart01 (as I
just checked) but I guess it didn't cover the problems you just found.

I was not aware of the legend entry breakage, so most likely it's still broken.
 And I'm not aware of any other bugs that may be in koheichart01.  So, please go
ahead and fix any problems you find in that cws. :-)

Thanks!
Comment 52 IngridvdM 2009-02-13 15:02:14 UTC
Ok fine! :-)
So just drop me a note before you resync thus I stop commiting then ;-)
Comment 53 IngridvdM 2009-02-13 16:09:19 UTC
I found and fixed a layout problem with the new controls in case of stock chart.
Comment 54 IngridvdM 2009-02-17 10:16:53 UTC
Fixed another problem: Load the attached document 'HiddenSeries.ods' and double
click on the two charts. There should not be a space between the two series. And
there should not be a legend entry for Column E because it is hidden and should
not be plotted.
Comment 55 IngridvdM 2009-02-17 10:18:05 UTC
Created attachment 60240 [details]
example with wrong space and wrong legend entry
Comment 56 kyoshida 2009-02-17 16:27:31 UTC
Yup, verified both problems (and the fixes).  Thanks!
Comment 57 IngridvdM 2009-02-17 17:13:47 UTC
Fixed another problem within the range highlighting: Load the attached file
'HiddenSeries.ods'. Within the column chart select the second data point of the
first(blue) series. What should happen is that cell D9 gets highlighted.
I will look for more tomorrow. This feature spreads quite ugly over the chart
code as it affects potentially all places where ranges are used.
Comment 58 IngridvdM 2009-02-18 19:06:43 UTC
The feature 'Plot missing values' is broken by this CWS. The data comes already
wrong from the calc. m_aDataArray in ScChart2DataSequence contains wrong Items.
mfValue is zero where it should be NAN and mbIsValue is true where it should be
false.
I'll attach a document 'MissingValue.ods' to illustrate the problem.
@kohei, as this is calc code could you please have a look what is going wrong
there? Thanks!
Comment 59 IngridvdM 2009-02-18 19:10:02 UTC
Created attachment 60289 [details]
double click the chart to see the problem with missing values
Comment 60 IngridvdM 2009-02-18 19:20:31 UTC
I fixed another problem with that feature. The radio button 'Leave Gap' could
not be clicked anymore. When arranging controls side by side one must be careful
that the controls do not overlap otherwise the underlying control is not
reachable anymore. Those controls are sized and placed dynamically now.
Comment 61 kyoshida 2009-02-18 19:45:51 UTC
hmm...  That UI was working before though.  I remember I had to work on that to
dynamically relocate the controls...
Comment 62 kyoshida 2009-02-19 05:53:28 UTC
Ok.  The missing value treatment issue is now fixed.  The fix was very easy;
just changing the default values of Item's members to non-value was enough to
fix it.
Comment 63 IngridvdM 2009-02-19 10:24:49 UTC
@kohei, thanks for the fix. I thought also that changing the constructor
defaults might be sufficient. I found that the same error is contained in CWS
koheiformula02. I am going to transport the fix therein.
Regarding the UI - it was not working before my first checkins to this CWS. I
have that version installed still. The relocation was dynamically in y direction
already that is right but not in size and x direction. Well it is a pain to not
have a layout engine - sigh.
A little help here comes with the VCL debug options. You can activate them with
Ctrl-Shift-Alt-D. Check the check box 'Dialog' and the current size of the
controls will be displayed as green rectangle if you open a new dialog.
Comment 64 IngridvdM 2009-02-27 17:55:13 UTC
I found and fixed some more problems:
When copying a chart with hidden and not plotted data and pasting it into writer
or impress the hidden data is not hidden anymore. The nasty thing here is that
the error is not visible immediately - at least in impress. If you double click
the chart or start your presentation you will see that your copy looks different
than the original.
Another problem was with the trendline equation that is displayed in the status
bar. Open the attached document TrendlineEquation.ods and select the trend line.
Look at the text in the status bar. The formula should read f(x)=x and not
f(x)=0.6571x+0.1905.
Comment 65 IngridvdM 2009-02-27 17:57:07 UTC
Created attachment 60555 [details]
example for status bar text of selected trenline
Comment 66 IngridvdM 2009-02-27 18:03:48 UTC
It makes no sense to fix another place and another place and another place where
values from ranges are used, so I redesigned the interface for this feature. The
chart now controls the data provider and the data sequences which got a new
property 'IncludeHiddenCells'. If that property is set to false the hidden
values are not propagated to the chart.
Comment 67 kyoshida 2009-02-27 22:22:52 UTC
@iha: Yup, that design change makes sense.  Thanks for keeping me in the loop. 
Comment 68 IngridvdM 2009-03-04 16:41:18 UTC
Some more changes:
I changed the default. Now newly created charts do not display values from
hidden cells. This is what is expected for user scenarios with charts from
filtered data (see issue 81209).
The new check box now is only visible in case the data provider does support
this feature. So currently the check box is only visible if the chart does
retrieve its data from a spreadsheet.
I updated the specification accordingly and send it to the other i-Team member
for review:
http://specs.openoffice.org/chart/MissingValues.odt
There was also a bug with undo/redo after I changed the interface design. And
finally I removed some code that was originally introduced for this feature but
finally not used.
I hope thats it now - but waiting for review feedback.
Comment 69 IngridvdM 2009-03-16 11:55:09 UTC
Feedback from i-Team received: The text from the new check box should better be
'Include values from hidden cells' instead of 'Plot values from hidden cells' as
the latter text might be misinterpreted that 'only' values from hidden cells are
plotted.
->I updated specification and implementation accordingly.
Comment 70 IngridvdM 2009-03-16 15:32:17 UTC
I created issue 100231 to adapt the automatic test scripts for this feature.
This needs to be done within the same CWS as the feature implementation.
Comment 71 IngridvdM 2009-03-17 13:20:06 UTC
I found that after the interface redesign the import from xls did not work
anymore. I fixed this now. I also added the import from xlsx.
Comment 72 IngridvdM 2009-04-23 15:11:35 UTC
@kla, please verify in CWS koheichart01.
Comment 73 kla 2009-05-18 16:05:37 UTC
Created attachment 62346 [details]
TCS Display_of_hidden_cells.htm
Comment 74 kla 2009-05-18 16:08:08 UTC
Issue verified, TCS written
Comment 75 Regina Henschel 2009-05-30 20:07:03 UTC
*** Issue 102369 has been marked as a duplicate of this issue. ***
Comment 76 amy2008 2009-08-13 07:09:44 UTC
Verified in DEV300m54 on Ubuntu.
Closing