Skip to content

Commit

Permalink
Merge pull request #2404 from finos/scroll-panel-height
Browse files Browse the repository at this point in the history
Fix scroll panel height calculation bug
  • Loading branch information
texodus authored Oct 30, 2023
2 parents 335a0ce + d453b18 commit 9415075
Show file tree
Hide file tree
Showing 10 changed files with 137 additions and 45 deletions.
12 changes: 6 additions & 6 deletions rust/perspective-viewer/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 8 additions & 7 deletions rust/perspective-viewer/src/rust/components/column_selector.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ use super::containers::scroll_panel::*;
use super::containers::split_panel::{Orientation, SplitPanel};
use super::style::LocalStyle;
use super::viewer::ColumnLocator;
use crate::components::containers::scroll_panel_item::ScrollPanelItem;
use crate::custom_elements::ColumnDropDownElement;
use crate::dragdrop::*;
use crate::model::*;
Expand Down Expand Up @@ -240,16 +241,16 @@ impl Component for ColumnSelector {
active_classes.push("is-aggregated");
}

let size = 28.0f64.mul_add(
let size_hint = 28.0f64.mul_add(
(config.group_by.len()
+ config.split_by.len()
+ config.filter.len()
+ config.sort.len()) as f64,
224.0,
220.0,
);

let config_selector = html_nested! {
<ScrollPanelItem key={ "config_selector" } { size }>
<ScrollPanelItem key={ "config_selector" } { size_hint }>
<ConfigSelector
dragdrop={ &ctx.props().dragdrop }
session={ &ctx.props().session }
Expand All @@ -266,13 +267,13 @@ impl Component for ColumnSelector {
.enumerate()
.map(|(idx, name)| {
let ondragenter = ondragenter.reform(move |_| Some(idx));
let size = if named_count > 0 { 48.0 } else { 28.0 };
let size_hint = if named_count > 0 { 50.0 } else { 28.0 };
named_count = named_count.saturating_sub(1);
let key = name.get_name().map(|x| x.to_owned()).unwrap_or_else(|| format!("__auto_{}__", idx));
let column_dropdown = self.column_dropdown.clone();
let is_editing = matches!(&ctx.props().selected_column, Some(ColumnLocator::Plain(x)) if x == &key);
html_nested! {
<ScrollPanelItem key={ key } { size }>
<ScrollPanelItem key={ key } { size_hint }>
<ActiveColumn
{ idx }
{ name }
Expand Down Expand Up @@ -300,7 +301,7 @@ impl Component for ColumnSelector {
html_nested! {
<ScrollPanelItem
key={ vc.name }
size={ 28.0 }>
size_hint={ 28.0 }>
<InactiveColumn
{ idx }
visible={ vc.is_visible }
Expand All @@ -324,7 +325,7 @@ impl Component for ColumnSelector {
};

let add_column = html_nested! {
<ScrollPanelItem key={ "__add_expression__" } size={ size }>
<ScrollPanelItem key={ "__add_expression__" } size_hint={ size }>
<AddExpressionButton
on_open_expr_panel={ &ctx.props().on_open_expr_panel }
selected_column={ ctx.props().selected_column.clone() }>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ mod symbol;
use yew::{function_component, html, Html, Properties};

use crate::components::column_settings_sidebar::style_tab::column_style::ColumnStyle;
use crate::config::plugin::{PluginAttributes, PluginConfig};
use crate::config::Type;
use crate::custom_events::CustomEvents;
use crate::renderer::Renderer;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ mod types;
use std::rc::Rc;

use itertools::Itertools;
use serde::{Deserialize, Serialize};
use yew::{html, Html, Properties};

use self::types::{SymbolConfig, SymbolKVPair};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ pub mod kvpair;
pub mod radio_list;
pub mod radio_list_item;
pub mod scroll_panel;
pub mod scroll_panel_item;
pub mod select;
pub mod sidebar;
pub mod split_panel;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,31 +21,11 @@ use web_sys::Element;
use yew::prelude::*;
use yew::virtual_dom::VChild;

use super::scroll_panel_item::ScrollPanelItem;
use crate::components::style::LocalStyle;
use crate::utils::*;
use crate::*;

#[derive(PartialEq, Properties)]
pub struct ScrollPanelItemProps {
pub size: f64,
pub children: Children,
}

pub struct ScrollPanelItem {}

impl Component for ScrollPanelItem {
type Message = ();
type Properties = ScrollPanelItemProps;

fn create(_ctx: &Context<Self>) -> Self {
Self {}
}

fn view(&self, ctx: &Context<Self>) -> Html {
ctx.props().children.iter().collect()
}
}

pub struct ScrollPanel {
viewport_ref: NodeRef,
viewport_height: f64,
Expand Down Expand Up @@ -96,7 +76,7 @@ impl ScrollPanelProps {
fn total_height(&self) -> f64 {
self.children
.iter()
.map(|x| x.props.size)
.map(|x| x.props.get_size())
.reduce(|x, y| x + y)
.unwrap_or_default()
}
Expand Down Expand Up @@ -149,12 +129,12 @@ impl ScrollPanel {
.iter()
.enumerate()
.find_or_last(|(i, x)| {
if offset + x.props.size < scroll_top {
if offset + x.props.get_size() < scroll_top {
start_node = *i + 1;
start_y = offset + x.props.size;
start_y = offset + x.props.get_size();
}

offset += x.props.size;
offset += x.props.get_size();
offset > scroll_top + self.viewport_height
})
.map(|x| x.0)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,113 @@
// ┏━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┓
// ┃ ██████ ██████ ██████ █ █ █ █ █ █▄ ▀███ █ ┃
// ┃ ▄▄▄▄▄█ █▄▄▄▄▄ ▄▄▄▄▄█ ▀▀▀▀▀█▀▀▀▀▀ █ ▀▀▀▀▀█ ████████▌▐███ ███▄ ▀█ █ ▀▀▀▀▀ ┃
// ┃ █▀▀▀▀▀ █▀▀▀▀▀ █▀██▀▀ ▄▄▄▄▄ █ ▄▄▄▄▄█ ▄▄▄▄▄█ ████████▌▐███ █████▄ █ ▄▄▄▄▄ ┃
// ┃ █ ██████ █ ▀█▄ █ ██████ █ ███▌▐███ ███████▄ █ ┃
// ┣━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┫
// ┃ Copyright (c) 2017, the Perspective Authors. ┃
// ┃ ╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌╌ ┃
// ┃ This file is part of the Perspective library, distributed under the terms ┃
// ┃ of the [Apache License 2.0](https://www.apache.org/licenses/LICENSE-2.0). ┃
// ┗━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━━┛

// Forked from https://github.com/AircastDev/yew-virtual-scroller (Apache 2.0)
// Adds support for Yew 0.19, auto-width and a simplified message structure.

#[cfg(debug_assertions)]
use std::cell::Cell;
#[cfg(debug_assertions)]
use std::rc::Rc;

use yew::prelude::*;

#[derive(PartialEq, Properties)]
pub struct ScrollPanelItemProps {
/// The expected size of this component in pixels. Calculating this value
/// ahead of time makes it easier to implement a high-performance virtual
/// renderer without resorting to weird tricks, _but_ checking that this
/// hint is correct is nearly as expensive. So, we only generate the
/// validation code in debug builds.
pub size_hint: f64,
pub children: Children,

#[cfg(debug_assertions)]
#[prop_or_default]
measured_size: Rc<Cell<Option<f64>>>,
}

impl ScrollPanelItemProps {
#[cfg(debug_assertions)]
pub fn get_size(&self) -> f64 {
self.measured_size.get().unwrap_or(self.size_hint)
}

#[cfg(not(debug_assertions))]
pub fn get_size(&self) -> f64 {
self.size_hint
}
}

#[cfg(debug_assertions)]
pub struct ScrollPanelItem {
node: NodeRef,
}

#[cfg(not(debug_assertions))]
pub struct ScrollPanelItem {}

impl Component for ScrollPanelItem {
type Message = ();
type Properties = ScrollPanelItemProps;

#[cfg(debug_assertions)]
fn create(ctx: &Context<Self>) -> Self {
ctx.props().measured_size.set(Some(ctx.props().size_hint));
Self {
node: NodeRef::default(),
}
}

#[cfg(not(debug_assertions))]
fn create(_ctx: &Context<Self>) -> Self {
Self {}
}

#[cfg(debug_assertions)]
fn view(&self, ctx: &Context<Self>) -> Html {
html! {
<div class="debug-size-wrapper" style="display:grid" ref={ self.node.clone() }>
{ for ctx.props().children.iter() }
</div>
}
}

#[cfg(not(debug_assertions))]
fn view(&self, ctx: &Context<Self>) -> Html {
html! {
{ for ctx.props().children.iter() }
}
}

#[cfg(debug_assertions)]
fn changed(&mut self, ctx: &Context<Self>, _old: &Self::Properties) -> bool {
ctx.props().measured_size.set(Some(ctx.props().size_hint));
true
}

#[cfg(debug_assertions)]
fn rendered(&mut self, ctx: &Context<Self>, first_render: bool) {
if first_render {
let elem = self.node.cast::<web_sys::HtmlElement>().unwrap();
let new_height = elem.get_bounding_client_rect().height();
if ctx.props().measured_size.get() != Some(new_height) {
tracing::warn!(
"ScrollPanel size_hint does not match element size: {} != {}",
ctx.props().size_hint,
new_height
);
web_sys::console::warn_1(&elem);
ctx.props().measured_size.set(Some(new_height));
}
}
}
}
3 changes: 1 addition & 2 deletions rust/perspective-viewer/src/rust/model/export_method.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ use std::rc::Rc;
use yew::prelude::*;

use crate::js::*;
use crate::*;

#[derive(Clone, Copy, Eq, PartialEq)]
pub enum ExportMethod {
Expand Down Expand Up @@ -90,7 +89,7 @@ impl From<ExportFile> for Html {
None
};

html_template! {
html! {
<code class={ class }>
{ x.name }
{ x.method.as_filename() }
Expand Down
6 changes: 3 additions & 3 deletions rust/perspective-viewer/tasks/bundle/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,6 @@ path = "main.rs"
bench = false

[dependencies]
flate2 = "1.0.20"
wasm-bindgen-cli-support = "0.2.84"
wasm-opt = "0.112.0"
flate2 = "1.0.28"
wasm-bindgen-cli-support = "0.2.87"
wasm-opt = "0.116.0"
Binary file modified tools/perspective-test/results.tar.gz
Binary file not shown.

0 comments on commit 9415075

Please sign in to comment.