fix gradient values #2384
fix gradient values #2384
Conversation
ada-x64
force-pushed
the
bugs/number-styles-gradient-values
branch
from
85bfa28 to
30e56c6
Compare
ada-x64
force-pushed
the
bugs/number-styles-gradient-values
branch
from
30e56c6 to
a800ac7
Compare
| if let Some(view) = ctx.props().view.clone() && let Some(column_name) = ctx.props().column_name.clone() { | ||
| ctx.link().send_future(async { | ||
| let view = view.unchecked_into::<JsPerspectiveView>(); | ||
| let min_max = view._get_min_max(column_name.into()).await.unwrap(); |
There was a problem hiding this comment.
The _ prefix in the extern "C" functions is meant to indicate a place where the native wasm_bindgen type support is insufficient, and that this method should not be made public or called directly. I would argue in this case that, as this function always returns the JavaScript-equivalent of a 2-tuple of f64, that we should add fn get_min_max(col: &str) -> (f64, f64) to the the impl JsPerspectiveView block in perspective.js extern module, such that consumers of this API need not juggle wasm_bindgen typecasting magic as below at the callsite.
As a general principal I think, we want to try to contain wasm_bindgen types to dedicated FFI/extern modules except in cases where it impacts performance.
| self.config.bg_gradient = None; | ||
| } | ||
| }; | ||
|
|
||
| self.dispatch_config(ctx); | ||
| false | ||
| } | ||
| NumberColumnStyleMsg::DefaultGradientChanged(gradient) => { |
There was a problem hiding this comment.
Distributing the default value logic across multiple files like means we have the of the complexity of the default_config/config design, but none of the decoupling. We should instead IMO make the default_config() getter an async method which can query it's container.
| html_template! { | ||
| <div class="item_title">{title.clone()}</div> | ||
| <div class="style_contents"> | ||
| <NumberColumnStyle { config } {default_config} {on_change} /> | ||
| <NumberColumnStyle column_name={column_name.clone()} view={view.clone()} { config } {default_config} {on_change} /> |
There was a problem hiding this comment.
Don't bind and pass View - pass the state object Session and request the view immediately where needed. This doesn't matter as much for this change due to the ephemeral nature of this component, but for components that persist between query changes, View can not only go stale, any remaining stale references will segfault if you try to call them.
Bugfix
Number style gradient values were not being populated. This uses the same code as an older version, but reimplemented in rust.
Testing
No tests were added or changed.
Screenshots (if appropriate)
Checklist
CONTRIBUTING.mdand followed its Guidelines