-
Notifications
You must be signed in to change notification settings - Fork 64
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
Histogram / Category widget #822
Histogram / Category widget #822
Conversation
I've also taken the chance to fix the always present legends panel, and given it a readable title. |
@@ -1,11 +1,11 @@ | |||
{% include 'utils/format.js.j2' %} | |||
|
|||
function renderWidget(layer, widget, variable, layerIndex, widgetIndex=0) { | |||
const element = document.querySelector(`#layer${layerIndex}_widget${widgetIndex}-${widget.name}`); | |||
const element = widget.element; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😍
} | ||
else: | ||
return {} | ||
|
||
def has_variable(self): | ||
return self._type == 'formula' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM!! Great work ✨ As we've talked, we'll have to define when a widget has to be updated when interacting with the visualization, but we can merge it and continue with this refactor in next issues 👍
Fix #811
Fix #810
Fix #771
I added some features to the widget class. We'll need some options to pass down to the bridge, and we might want to add some extra options for the components themselves.
For instance, the category widget should probably have some height limitations and a way to tweak max visible categories.