Merge pull request #1450 from finos/fixes-23c

Expression editor bug fixes

packages/perspective-jupyterlab/src/less/index.less

@@ -1,5 +1,5 @@
 @import "~@finos/perspective-viewer/src/less/fonts.less";
-@import (reference) "~@finos/perspective-viewer/src/themes/material-dense.less";
+@import "~@finos/perspective-viewer/src/themes/material-dense.less";
 @import (reference) "~@finos/perspective-viewer/src/themes/material-dense.dark.less";
 
 div.PSPContainer,

packages/perspective-viewer/test/results/linux.docker.json

@@ -54,7 +54,7 @@
     "Computed_Expressions_On_restore,_computed_expressions_in_classic_syntax_are_parsed_correctly_": "563f8c45e0ec32bc4b07d30ed5f0086e",
     "superstore_adds_computed_column_via_attribute": "f2fa526d6f0c168a47ab272bb7ae5771",
     "superstore_user_defined_aggregates_maintained_on_computed_columns": "c7aa2902feb512ceb1e1487de80878ff",
-    "__GIT_COMMIT__": "55e80696b3f95ff9ce948fff91ae690895d9819e",
+    "__GIT_COMMIT__": "5f16bb751b9f39684c6f198010c10b85acec7ae8",
     "blank_Handles_reloading_with_a_schema_": "2f23d1416fc97e07a3a21e318411e0d1",
     "superstore_doesn_t_leak_tables_": "5bb762b2860eb5af067bb83afbca3264",
     "superstore_doesn_t_leak_elements_": "b8a3a84406fede4823cd51e07b817752",
@@ -94,17 +94,17 @@
     "filters_less_than_ISO_string_on_datetime_column": "b7a0f123b4d5dea1b710aeea7e61572a",
     "filters_less_than_US_locale_string_on_datetime_column": "13e4f63b1060272bff4f01382e565e82",
     "blank_Should_load_a_promise_to_a_table_": "a3841665c5fc438c5e959c9858540744",
-    "Expressions_click_on_add_column_button_opens_the_expression_UI_": "b909fc09ef2d2062896bf0c5d902589f",
+    "Expressions_click_on_add_column_button_opens_the_expression_UI_": "1673913b2dd7fff02ce29d30b5b3e753",
     "Expressions_click_on_close_button_closes_the_expression_UI_": "a02605db902dc06ea824fb013117ae6b",
-    "Expressions_An_expression_with_unknown_symbols_should_disable_the_save_button": "c79b2b1ea739fc3f4cfbb155c4768bf0",
-    "Expressions_A_type-invalid_expression_should_disable_the_save_button": "beca6e3c90f2032a24f95f7e7e175cee",
-    "Expressions_An_expression_with_invalid_input_columns_should_disable_the_save_button": "b53c4672d455f775848457078e05c755",
+    "Expressions_An_expression_with_unknown_symbols_should_disable_the_save_button": "cf709215e66f549c712c9d799bd08300",
+    "Expressions_A_type-invalid_expression_should_disable_the_save_button": "7c858cca0a50948be84eb4ab9807e0c9",
+    "Expressions_An_expression_with_invalid_input_columns_should_disable_the_save_button": "a901080b11656a364ebfb60064a3453f",
     "Expressions_Should_show_the_help_panel": "dc16ac2736a46205fa13a30cc2842510",
     "Expressions_Non-aliased_expressions_should_have_autogenerated_alias": "0478c89a614c402083c7bf0eca48a66d",
     "Expressions_Should_skip_if_trying_to_set_an_expression_without_an_alias": "d13e8556321b0a8bbbf5122ce4f95754",
-    "Expressions_Should_prevent_saving_a_duplicate_expression_alias": "f4975b44f2cebb81d4a6a99901f3c746",
-    "Expressions_Should_not_prevent_saving_a_duplicate_expression_with_a_different_alias": "3c27a1c305c2e5ce4c3e44c055aa15dd",
-    "Expressions_Should_prevent_saving_a_duplicate_expression": "fb0258131aa8514a8e05db26aeb01c14",
+    "Expressions_Should_prevent_saving_a_duplicate_expression_alias": "8d5917cbb202037bb700aaff0ca682b0",
+    "Expressions_Should_not_prevent_saving_a_duplicate_expression_with_a_different_alias": "22f666bc10293abc7a46b1741f56bf1f",
+    "Expressions_Should_prevent_saving_a_duplicate_expression": "fd34c704fe673e00dcb61437b719fa4e",
     "Expressions_Removing_expressions_should_reset_active_columns,_pivots,_sort,_and_filter_": "e31a7da6f65bf5edc7d6582a4514a069",
     "Expressions_Resetting_the_viewer_with_expressions_should_place_columns_in_the_inactive_list_": "17722593bb3bc9564a29edf1859c6951",
     "Expressions_Resetting_the_viewer_with_columns_in_active_columns_should_reset_columns_but_not_delete_columns_": "17722593bb3bc9564a29edf1859c6951",
@@ -114,7 +114,7 @@
     "Expressions_saving_without_an_expression_should_fail_as_button_is_disabled_": "462c0e101f72c021987def75335a967c",
     "Expressions_saving_a_single_expression_should_add_it_to_inactive_columns_": "28d815bfccfcd54f6f2ed6b683d86055",
     "Expressions_saving_multiple_expressions_should_add_it_to_inactive_columns_": "74b4a63079bc47b9809a92967335d3bf",
-    "Expressions_Expression_columns_should_persist_when_new_views_are_created_": "6aa47661329b23d7f0fd8f243ee89ca2",
+    "Expressions_Expression_columns_should_persist_when_new_views_are_created_": "7996884d2c469544e19d2b9cd75a5d0e",
     "Expressions_Expression_columns_should_persist_when_new_columns_are_added_": "951b31155154d97badbda8dea3a28feb",
     "Expressions_aggregates_by_expression_column_": "e7ba48d18a68dd96282d79fc0ba609bd",
     "Expressions_sets_column_attribute_with_expression_column_": "516a4292d1784b97fe4de8fa37483e4f",

packages/perspective/src/js/perspective.js

@@ -1413,8 +1413,6 @@ export default function(Module) {
             // bound using `value_object` in embind so no need to manually
             // convert to Object, or call delete() as memory is auto-managed.
             const error_object = expression_errors.get(alias);
-
-            console.log(error_object);
             validated.errors[alias] = error_object;
         }
 
@@ -1423,7 +1421,6 @@ export default function(Module) {
         expression_errors.delete();
         expression_schema.delete();
         validation_results.delete();
-
         return validated;
     };
 

rust/perspective-vieux/Cargo.toml

@@ -20,7 +20,7 @@ js-intern = "0.3.1"
 num-format = "0.4.0"
 serde = { version = "1.0", features = ["derive"] }
 serde_json = "1.0.59"
-wasm-bindgen = { version = "=0.2.74" , features = ["serde-serialize"] }
+wasm-bindgen = { version = "=0.2.70" , features = ["serde-serialize"] }
 wasm-bindgen-futures = "0.4.20"
 yew = { git = "https://github.com/yewstack/yew", rev = "c51ab7f" } # "0.17.4"
 

rust/perspective-vieux/src/less/expression-editor.less

@@ -24,8 +24,7 @@
     #monaco-container {
         width:400px;
         height:200px;
-        resize:both;
-        overflow:auto;
+        overflow: visible;
         &::-webkit-resizer {
             appearance: var(--monaco-container--appearance, auto);
         }

rust/perspective-vieux/src/rust/components/expression_editor.rs

@@ -13,6 +13,7 @@ use crate::utils::perspective::*;
 use crate::utils::*;
 
 use std::cell::RefCell;
+use std::iter::FromIterator;
 use std::rc::Rc;
 use wasm_bindgen::{prelude::*, JsCast};
 use wasm_bindgen_futures::future_to_promise;
@@ -187,13 +188,13 @@ impl ExpressionEditor {
         let expr = editor.get_value();
         (self.props.on_validate)(true);
         let model = editor.get_model();
-        let arr = js_sys::Array::new();
-        let msg = match self.props.session.validate_expr(expr).await? {
-            None => true,
+        let (msg, arr) = match self.props.session.validate_expr(expr).await? {
+            None => (true, js_sys::Array::new()),
             Some(err) => {
                 let marker = error_to_market(err);
-                arr.push(&JsValue::from_serde(&marker).unwrap());
-                false
+                let args = JsValue::from_serde(&marker).unwrap();
+                let arr = js_sys::Array::from_iter([args].iter());
+                (false, arr)
             }
         };
 

rust/perspective-vieux/src/rust/custom_elements/modal.rs

@@ -49,19 +49,12 @@ fn calc_page_position(target: &HtmlElement) -> Result<(u32, u32), JsValue> {
     let mut left = 0;
     let mut elem = target.clone().unchecked_into::<HtmlElement>();
     while !elem.is_undefined() {
-        let is_sticky = match web_sys::window().unwrap().get_computed_style(&elem)? {
-            Some(x) => x.get_property_value("position")? == "sticky",
-            _ => false,
-        };
-
         top += elem.offset_top();
         left += elem.offset_left();
         elem = match elem.offset_parent() {
             Some(elem) => {
-                if is_sticky {
-                    top -= elem.scroll_top();
-                    left -= elem.scroll_left();
-                }
+                top -= elem.scroll_top();
+                left -= elem.scroll_left();
                 elem.unchecked_into::<HtmlElement>()
             }
             None => match elem.dyn_into::<ShadowRoot>() {

rust/perspective-vieux/src/rust/session.rs

@@ -12,12 +12,12 @@ mod view_subscription;
 
 use crate::session::view_subscription::*;
 use crate::utils::perspective::*;
-use js_sys::Array;
 pub use view_subscription::TableStats;
 
 use copy::*;
 use download::*;
 use std::cell::RefCell;
+use std::iter::FromIterator;
 use std::ops::Deref;
 use std::rc::Rc;
 use wasm_bindgen::prelude::*;
@@ -67,7 +67,7 @@ impl Session {
         Ok(JsValue::UNDEFINED)
     }
 
-    /// The `table`'s unique column names.  This value is not 
+    /// The `table`'s unique column names.  This value is not
     pub async fn get_column_names(self) -> Result<Vec<String>, JsValue> {
         match self.borrow().table.as_ref() {
             None => Ok(vec![]),
@@ -84,23 +84,26 @@ impl Session {
         }
     }
 
-    pub async fn validate_expr(self, expr: JsValue) -> Result<Option<PerspectiveValidationError>, JsValue> {
-        let arr = Array::new();
-        arr.push(&expr);
-        let result = self
+    /// Validate an expression string (as a JsValue since it comes from `monaco`),
+    /// and marshall the results.
+    pub async fn validate_expr(
+        self,
+        expr: JsValue,
+    ) -> Result<Option<PerspectiveValidationError>, JsValue> {
+        let arr = js_sys::Array::from_iter([expr].iter());
+        let errors = self
             .borrow()
             .table
             .as_ref()
             .unwrap()
             .validate_expressions(arr)
-            .await?;
+            .await?
+            .errors();
 
-        let errors = result.errors();
         let error_keys = js_sys::Object::keys(&errors);
         if error_keys.length() > 0 {
             let js_err = js_sys::Reflect::get(&errors, &error_keys.get(0))?;
-            let error: PerspectiveValidationError = js_err.into_serde().unwrap();
-            Ok(Some(error))
+            Ok(Some(js_err.into_serde().unwrap()))
         } else {
             Ok(None)
         }

rust/perspective-vieux/src/rust/session/view_subscription.rs

@@ -40,7 +40,7 @@ pub struct ViewSubscription {
 impl ViewSubscriptionData {
     /// TODO Use serde to serialize the full view config, instead of calculating
     /// `is_pivot` here.
-    pub async fn update_view_stats(self) -> Result<JsValue, JsValue> {
+    async fn update_view_stats(self) -> Result<JsValue, JsValue> {
         let config = self.view.get_config().await?;
         let num_rows = self.table.size().await? as u32;
         let virtual_rows = self.view.num_rows().await? as u32;