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

Added new responsive charts and new color settings #143

Closed
wants to merge 12 commits into from
Closed

Added new responsive charts and new color settings #143

wants to merge 12 commits into from

Conversation

josesaranda
Copy link

In this commit you can view a new example of how to use the new config variables fillColor and responsive.

Screen Recording 2019-08-08 at 03 02 27 13 PM

@domoritz domoritz self-requested a review August 8, 2019 13:22
Copy link
Member

@domoritz domoritz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the PR.

As I understand it, this adds support for responsive charts and changing the colors of bars.

Instead of just setting the fill color in the falcon config, please add support for full Vega configs (https://vega.github.io/vega/docs/config/). This will allow for much greater levels of customization.

To make charts responsive, please add some code to the views to listen to changes of the container size with https://vega.github.io/vega/docs/expressions/#containerSize. This will let Vega handle resizing instead.

@domoritz
Copy link
Member

domoritz commented Aug 8, 2019

I created two new issues to track these improvements. #144 and #145. It would be great to address them separately.

Again, thank you for working on Falcon. It's great to have these improvements come in.

@josesaranda
Copy link
Author

josesaranda commented Aug 8, 2019

I think this support require a lot of effort. You can accept the PR for a minor version. (i.e: 0.14.4)
I would be very grateful

@domoritz
Copy link
Member

domoritz commented Aug 8, 2019

Which one? Theming or responsive charts?

Does your PR work with charts in multiple columns? It looks like every chart spans the whole width in your video.

@josesaranda
Copy link
Author

Both. My PR works in multiple columns if you describe your container rewriting utils.ts createElement function and stylish the App.scss. Please see the example and try to change the layout. Then i will be grateful if you approve this PR.

@josesaranda
Copy link
Author

josesaranda commented Aug 9, 2019

Following the conversation on issue #144 I have forked the repo and published this feature in a new npm package called shapelets-falcon. I am keeping the links to the main repo and referenced your work.

@domoritz
Copy link
Member

domoritz commented Aug 9, 2019

Excellent. Thank you for sending the pull request! It's great to see that Falcon is being used and what things would make it better (theming and responsiveness).

I'm going to close this PR but I will keep the issues that were inspired by this PR open.

@domoritz domoritz closed this Aug 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants