DM-6287 Charts refactoring #118

tgoldina · 2016-07-14T15:18:23Z

Moved chart-related code to charts package
Converted React components to ES6 classes
Reorganized controllers to store chart data (xyplot and histogram) under charts space

Fixed bugs

missing chart mount action, when a chart is removed and then recreated on the same table
undefined shows as a label when no server call is necessary

…, converted React components to ES6 classes, fixed bug when undefined shows as a label.

…der charts space, fixed bug - missing chart mount action, when a chart is removed and then recreated on the same table.

loitly · 2016-07-14T16:02:09Z

src/firefly/js/charts/ChartsCntlr.js

-                        return updateSet(state, ['tbl', tblId], omit(state['tbl'][tblId], [uChartId]));
+                if (has(state, [tblId, uChartId])) {
+                    if (Object.keys(state[tblId]).length > 1) {
+                        return updateSet(state, tblId, omit(state[tblId], [uChartId]));


WebUtil.updateDelete may do the same thing.

loitly · 2016-07-14T16:59:20Z

suggestion for xyplot optons panel:
move 'apply', 'clear', and 'reset' to the top and make it not scrollable. as is, in demo page where the buttons are cleanly hidden, it's a bit confusing what to do next after you've enter the options.

loitly · 2016-07-14T17:05:28Z

I did minimal spot testing and it looks good.

ejoliet · 2016-07-14T18:29:23Z

Randomly checking, i've spotted an issue:

controls (selection, filter) works but zoom doesn't work in firefox or safari (works in chrome though)

ejoliet · 2016-07-14T18:31:32Z

Another small one: 'Reset' doesn't reset anything - what is 'reset' suppose to do?

ejoliet · 2016-07-14T20:21:48Z

Last one, related to top container bar, maybe less important because it appears in the playground environment but anyway:
When i open the 'Charts' dialog panel and select a column from 'Col' link, clicking 'OK' made the 'Charts' dialog to close when i expect to continue and select the other column and only then i can 'Apply' my selections and close the dialog to show the updated plot. What do you think?

xiuqin · 2016-07-14T20:41:27Z

I agree with Emmanuel on this one.
It should behave the same as the option pop-up when clicking on the ‘gear’ icon.

Xiuqin

On Jul 14, 2016, at 1:21 PM, Emmanuel Joliet notifications@github.com wrote:

Last one, related to top container bar, maybe less important because it appears in the playground environment but anyway:
When i open the 'Charts' dialog panel and select a column from 'Col' link, clicking 'OK' made the 'Charts' dialog to close when i expect to continue and select the other column and only then i can 'Apply' my selections and close the dialog to show the updated plot. What do you think?

—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

xiuqin · 2016-07-14T21:01:52Z

zoom does not work in my Chrome browser.

Xiuqin

On Jul 14, 2016, at 11:29 AM, Emmanuel Joliet notifications@github.com wrote:

Randomly checking, i've spotted an issue:

controls (selection, filter) works but zoom doesn't work in firefox or safari (works in chrome though)
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub, or mute the thread.

xiuqin · 2016-07-14T21:05:32Z

This is a big change from the current OPS version.
Lets hold off until we hear from the users.

Xiuqin

On Jul 14, 2016, at 9:59 AM, loitly notifications@github.com wrote:

move 'apply', 'clear', and 'reset' to the top and make it not scrollable. as is, in demo page where the buttons are cleanly hidden, it's a bit confusing what to do next after you've enter the options.

tgoldina · 2016-07-14T22:48:06Z

Reset is resetting the form values to match current parameters of the
current plot. Clear clears the values to some reasonable default so that
you can define a new chart.

Hopefully, I have fixed the zoom issue.

2016-07-14 11:31 GMT-07:00 Emmanuel Joliet notifications@github.com:

Another small one: 'Reset' doesn't reset anything - what is 'reset'
suppose to do?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#118 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ALU3QPUSYuSLtkAnY2p6JfNLWotdvWwvks5qVoCEgaJpZM4JMium
.

tgoldina · 2016-07-14T22:53:01Z

This one is a bug, related to table actions affecting layout. The layout is
being changed on TABLE_REMOVE. It has the same nature as expanded layout
being changed to unexpanded on table filter or sort. I have reported it in
DM-6909.

2016-07-14 13:21 GMT-07:00 Emmanuel Joliet notifications@github.com:

Last one, related to top container bar, maybe less important because it
appears in the playground environment but anyway:
When i open the 'Charts' dialog panel and select a column from 'Col' link,
clicking 'OK' made the 'Charts' dialog to close when i expect to continue
and select the other column and only then i can 'Apply' my selections and
close the dialog to show the updated plot. What do you think?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#118 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ALU3QB_bKtaJBFxsfdze4vgnLLaP2ICXks5qVppdgaJpZM4JMium
.

tgoldina · 2016-07-14T23:00:17Z

Hopefully, I have fixed zoom.

I have put the buttons on top for convenience, but they will scroll
away. It's only in side options and easy to change back. I could not figure
out the style to scroll just the form.

2016-07-14 15:53 GMT-07:00 Tatiana Goldina tgoldina@gmail.com:

This one is a bug, related to table actions affecting layout. The layout
is being changed on TABLE_REMOVE. It has the same nature as expanded layout
being changed to unexpanded on table filter or sort. I have reported it in
DM-6909.

2016-07-14 13:21 GMT-07:00 Emmanuel Joliet notifications@github.com:

Last one, related to top container bar, maybe less important because it
appears in the playground environment but anyway:
When i open the 'Charts' dialog panel and select a column from 'Col'
link, clicking 'OK' made the 'Charts' dialog to close when i expect to
continue and select the other column and only then i can 'Apply' my
selections and close the dialog to show the updated plot. What do you think?

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#118 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/ALU3QB_bKtaJBFxsfdze4vgnLLaP2ICXks5qVppdgaJpZM4JMium
.

tgoldina added 2 commits July 13, 2016 11:21

DM-6287 chart refactoring: moved chart-related code to charts package…

e15d4e2

…, converted React components to ES6 classes, fixed bug when undefined shows as a label.

DM-6287 chart refactoring: store chart data (xyplot and histogram) un…

9db4a45

…der charts space, fixed bug - missing chart mount action, when a chart is removed and then recreated on the same table.

loitly reviewed Jul 14, 2016
View reviewed changes

DM-6287 response to review comments

992036f

tgoldina merged commit cca2bc2 into dev Jul 14, 2016

tgoldina deleted the dm-6287_refactor branch July 15, 2016 13:20

DM-6287 Charts refactoring #118

DM-6287 Charts refactoring #118

Uh oh!

Conversation

tgoldina commented Jul 14, 2016

Uh oh!

loitly Jul 14, 2016

Choose a reason for hiding this comment

Uh oh!

loitly commented Jul 14, 2016

Uh oh!

loitly commented Jul 14, 2016

Uh oh!

ejoliet commented Jul 14, 2016

Uh oh!

ejoliet commented Jul 14, 2016

Uh oh!

ejoliet commented Jul 14, 2016

Uh oh!

xiuqin commented Jul 14, 2016

Uh oh!

xiuqin commented Jul 14, 2016

Uh oh!

xiuqin commented Jul 14, 2016

Uh oh!

tgoldina commented Jul 14, 2016

Uh oh!

tgoldina commented Jul 14, 2016

Uh oh!

tgoldina commented Jul 14, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants