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

bug in scrolling clusterize table if it has different row heights #62

Closed
ebissolo opened this issue Feb 17, 2016 · 7 comments
Closed

bug in scrolling clusterize table if it has different row heights #62

ebissolo opened this issue Feb 17, 2016 · 7 comments

Comments

@ebissolo
Copy link

If is enabled break-word css property in td elements and I put a number of words that go out of td bound it causes the end line and the current row height is modified. Then if I try to scroll, the next cluster is updated but not in right moment.

@ebissolo ebissolo reopened this Feb 17, 2016
@NeXTs
Copy link
Owner

NeXTs commented Feb 17, 2016

Please provide codepen.io example

This is not a bug, it's mostly like a plugin's restriction of rows height. Rows should be same height or at least approximately the same height. See first item at FAQ.

It behaves this way because clusterize picks height from one of rows during initialisation and uses this height for further operations. It only suggests height of cluster (chunk of rows) based on previously picked row height. Clusterize isn't aware of actual full-list height.

That's why rows should have similar height.

I will be happy to hear your suggestions regarding solution to this task :)

@ebissolo
Copy link
Author

ok... in my opinion the best thing to do is to avoid this bahavior of clusterize. Adding in clusterize.css a word-wrap: nowrap; in td elements of table it keeps rows height unchanged.

@NeXTs
Copy link
Owner

NeXTs commented Feb 22, 2016

It will work for you but may break someone other's expectations.
Not everyone needs one-line rows in their tasks.
So no silver bullet here IMO.

It's great if word-wrap: nowrap; works for you and solves your task. Use it and let this thread be example of possible solution for others who faces similar problem.

@NeXTs NeXTs closed this as completed Feb 25, 2016
@de-robat
Copy link

de-robat commented Apr 21, 2017

I've been tinkering around with providing an array of row heights along with the markup array and then using this array of heights for most of the calculations done for determining cluster postion and so on. This works really well.

This approach may help others who do know theire row heights in advance.

@dfoteinakis
Copy link

@de-robat Do you have a patch of your fix for passing the row heights? Thanks!

@de-robat
Copy link

de-robat commented Jul 13, 2017

nothing generic unfortunatley. I wanted to prepare a MR for the root project but haven't found the time to do so by now.

What i basically did, is extending some of the original functions in clusterize.js:

i changed the update function to take in an array of row heights:

self.update = function(new_rows, row_heights) {
	rows = isArray(new_rows) ? new_rows : [];
	self.row_heights = row_heights;
	var scroll_top = self.options.scroll_top_getter();
	// fixes #39
	var totalHeight;
	if (self.row_heights) {
		totalHeight = self.row_heights.reduce(function(a, b) {
			return a + b
		}, 0);
	} else {
		totalHeight = rows.length * self.options.item_height;
	}

	if (totalHeight < scroll_top) {
		self.options.scroll_top_setter(0);
		last_cluster = 0;
	}
	self.insertToDOM(rows, cache);
	self.options.scroll_top_setter(scroll_top)
}

then i adjusted all the places where the heights calculations are done do respect this array if present:

self.getScrollProgress = function() {
	var totalHeight;
	if (this.row_heights) {
		totalHeight = this.row_heights.reduce(function(a, b) {
			return a + b
		}, 0);
	} else {
		totalHeight = rows.length * this.options.item_height;
	}
	return ((this.options.scroll_top / totalHeight) * 100) || 0;
}

...

getClusterNum: function() {
	this.options.scroll_top = this.options.scroll_top_getter();
	if (this.row_heights) {
		//if we know the exact heights of each row we determine the current cluster by getting the currently drawn rows
		//deviding them by rows per cluster
		var currentHeight = 0;
		for (var rowNum = 0, len = this.row_heights.length; rowNum < len - 1; rowNum++) {
			currentHeight += this.row_heights[rowNum];
			if (currentHeight >= this.options.scroll_top) {
				//this is the row that determines the activley shown cluster
				return Math.floor(rowNum / (this.options.rows_in_cluster - this.options.rows_in_block)) || 0;
			}
		}
	}
	return Math.floor(this.options.scroll_top / (this.options.cluster_height - this.options.block_height)) || 0;
}

...

generate: function(rows, cluster_num) {
	var opts = this.options,
		rows_len = rows.length;
	if (rows_len < opts.rows_in_block) {
		return {
			top_offset: 0,
			bottom_offset: 0,
			rows_above: 0,
			rows: rows_len ? rows : this.generateEmptyRow()
		}
	}
	var items_start = Math.max((opts.rows_in_cluster - opts.rows_in_block) * cluster_num, 0),
		items_end = items_start + opts.rows_in_cluster,
		top_offset = Math.max(items_start * opts.item_height, 0),
		bottom_offset = Math.max((rows_len - items_end) * opts.item_height, 0),
		this_cluster_rows = [],
		rows_above = items_start;

	if (this.row_heights) {
		top_offset = this.row_heights.slice(0, items_start).reduce(function(a, b) {
			return a + b;
		}, 0);
		bottom_offset = this.row_heights.slice(items_end - 1).reduce(function(a, b) {
			return a + b;
		}, 0);
	}

	if (top_offset < 1) {
		rows_above++;
	}
	for (var i = items_start; i < items_end; i++) {
		rows[i] && this_cluster_rows.push(rows[i]);
	}
	return {
		top_offset: top_offset,
		bottom_offset: bottom_offset,
		rows_above: rows_above,
		rows: this_cluster_rows
	}
}

To do all this properly and put up a MR for the root repo one should probably go into the direction of extension points. I'd be willing to help revise it if you find the time to do so.

@katesclau
Copy link

Fell in the same but. Unfortunately, for our case, we won't know the row size previous to cluster update - so cluster height has to be dynamic.

Current workaround we've adopt is to toggle white-space nowrap to normal on row selection. That gives us some way to display the data to user without issues on scrolling.

Yet, this is not the best outcome, as we intend to provide visibility to all data, all the time.

Any hints on how to update the cluster height after all row heights are updated?

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

No branches or pull requests

5 participants