vxsl commented 2 years ago

depends on EQWorks/chart-system#155

^ among generally implementing the below new features, EQWorks/chart-system#155 decouples subplot rendering, titles, and legends from plotly.js so we can have finer control. Thus, suggestions on how the legend should be styled, how the titles should be styled, etc are now welcome.

new viz-options constants to facilitate new features described below
state augmented to facilitate new features described below
- (especially genericOptions)
- showLegend has been moved from a specific option of pie to a genericOption
refreshed story sample configs to match new implementation
- these are produced dynamically, as they will be produced when persistence is introduced
the app now dynamically formats column names
- the "raw" column names are not replaced, just kept track of in parallel and made available to the adapter

new features

hue control

for now, just wanted the simplest color control. Will improve soon to match design. As mentioned in EQWorks/chart-system#155, the color scheme generation based on the baseColor is currently lacking as well. https://user-images.githubusercontent.com/53827672/150404955-d136a837-932c-4a40-b77e-1a4673d8157b.mp4

size control

https://user-images.githubusercontent.com/53827672/150405409-db4204ef-bc65-4f5a-b7e0-01ddc0431009.mp4

had to increase the package size-limit from 500KB to 600KB
- alternatively, could have excluded stories/sample-data.js from the size-limit calculation but I couldn't get the ignore option to work
- still, the major contributor to the size is plotly, even using the basic subset of the library, discussed in EQWorks/chart-system#150

github-actions[bot] commented 2 years ago

📚 Storybook preview (updated to 5aa8b9e2ccdf54397113a08c7865a52e5bc5da4e)

geoerika commented 2 years ago

@vxsl Will approve this, all the issues I raised here can be addressed in another PR, as well as anything @sallkall might want to have changed on design part. There are quite a few changes from both of us again on the common files for charts and map so I would really like to do a rebase of my branch before I add to the editor part for the map.

vxsl commented 2 years ago

@geoerika

The overflow of the selection list in the DropdownSelect which makes the control panel to change height. I saw a PR with the component in lumen-labs, not sure you addressed this.

Thanks for pointing that out, fix included in the next PR

The flickering of the charts when loading and changing containers needs also to be fixed. In my opinion it makes for a very poor UX.

Agreed, will probably just remove this mechanism, as it was intended to make it less choppy by reducing viz re-renders, but the flashing is super annoying. Maybe if I have time in the future I will take another stab at a more graceful solution to this problem.

I believe dataHasVariance has to be set resettable: true in the state model

Good catch, will test and include in the next PR

Will approve this, all the issues I raised here can be addressed in another PR, as well as anything @sallkall might want to have changed on design part. There are quite a few changes from both of us again on the common files for charts and map so I would really like to do a rebase of my branch before I add to the editor part for the map.

Sounds good. Merging now.

@sallkall

question - is this going to be the MVP editor view or should I reframe from giving design comments until you have the full-page widget editor view up?

As discussed yesterday we can wait :+1:

@woozyking

should this action reside within the same hook right below it? since the dependencies are exactly the same and that one isn't very bloated yet

Yes absolutely, good catch

we should prefer elaborate if/else statements over ternary when there is a nested logic

True, this is a bad habit of mine - at some point will refactor this custom hook to apply this across each memo

if we're in the mood for regex all the way

nice, going to replace the fn in next PR

also we can consider using css to transform text to capital

Unfortunately can't, as this logic is independent of the rendering library

EQWorks / widget-studio

[G2M] Widget/some new features #51

depends on EQWorks/chart-system#155

new features

grouping by value

group filtering

subplot title positioning