WowAce

Edit for future readers: All of this and other enhancements have been merged into the project code as of v4.0.3-beta1 (and later). If anyone else wants to contribute to the project, do so at its GitHub page: https://github.com/ddumont/lib-st/

---

@ddumont Thanks for putting together this library all those years ago. It's been pretty easy to integrate into my project. It's particularly nice that I'm able to access the underlying `.frame` object and re-styling it to fit my own GUI better. :-) Very nice stuff!

However, I noticed that the code for `IsRowVisible()` is 100%, completely and utterly broken. ;-) It's looking for `.data[]` row offsets (`realrow` argument) within the range of the FauxScrollingFrame's `.offset`, which uses a totally different numbering system.

The only solution is to scrap the old code, and write a loop which loops through all visible rows (in the range between `.offset` and `.offset + .displayRows`), and looks in the `.filtered[]` array to see what real row each of the visible FauxScrollingFrame rows are displaying.

I've done that rewrite and quadruple-tested it to ensure that it correctly checks all visible rows on screen, and detects whenever one of them is displaying data from the `realrow` you provided.

I also fixed the bugged naming in the exposed public API, since there was a typo (the public function was named `RowIsVisible` instead of `IsRowVisible`, even though docs and the internal code say it should have been the latter).

Personally, my reason for fixing this function is efficiency: Whenever the underlying data changes, I only need to trigger `:Refresh()` if the changed row is currently being displayed. And it's therefore MUCH faster to perform an `:IsRowVisible()` call (which quickly loops through the array of visible rows to check if any are displaying the modified row). Rather than always triggering a `:Refresh()`, which does much heavier work of looping through all visible rows and re-drawing them all...

With this code fix, it's possible for library users to check `:IsRowVisible(theRow)` and ONLY triggering your `:Refresh()` (GUI re-drawing) calls when actually needed. (Also note: When the GUI user scrolls the scrollbar, it automatically refreshes the entire view of visible rows by getting the latest data for each cell in the underlying `.data` table. So there's no need to trigger `:Refresh()` in this library for any other reason than when a modified row is currently being shown on screen. Hence why I fixed `:IsRowVisible()` to make that optimization work!).

(Last sidenote, in case my text above confuses someone: Users of this library must of course ALWAYS call `:SortData()` whenever you add/delete rows from your data! The `:Refresh()` usage/optimization I'm talking about is ONLY usable when row contents CHANGE without any rows being added/deleted. I hope all of this makes sense to the readers, if any newcomers stumble across my comment... ;-))

diff -ru a/Core.lua b/Core.lua
--- a/Core.lua	2018-10-29 19:34:26.000000000 +0100
+++ b/Core.lua	2018-10-29 19:33:57.000000000 +0100
@@ -592,9 +592,16 @@
 	-- @name IsRowVisible
 	-- @description Checks if a row is currently being shown
 	-- @usage st:IsRowVisible(realrow)
-	-- @thanks sapu94
 	local function IsRowVisible(self, realrow)
-		return (realrow > self.offset and realrow <= (self.displayRows + self.offset))
+		-- Scan through all on-screen rows, checking their real data row numbers for a match.
+		local firstVisibleIdx = 1 + (self.offset or 0);
+		local lastVisibleIdx = (firstVisibleIdx + self.displayRows) - 1;
+		for i=firstVisibleIdx,lastVisibleIdx do
+			if self.filtered[i] == realrow and realrow ~= nil then
+				return true;
+			end
+		end
+		return false;
 	end
 	
 	function ScrollingTable:CreateST(cols, numRows, rowHeight, highlight, parent)
@@ -629,7 +636,7 @@
 		st.GetCell = GetCell;
 		st.GetRow = GetRow;
 		st.DoCellUpdate = DoCellUpdate;
-		st.RowIsVisible = IsRowVisible;
+		st.IsRowVisible = IsRowVisible;
 		
 		st.SetFilter = SetFilter;
 		st.DoFilter = DoFilter;

In reply to ddumont:

st.RowIsVisible = IsRowVisible; -- Old name for backwards compatibility.
st.IsRowVisible = IsRowVisible;

The sorting.

"asc" is descending order, and "dsc" is ascending order.

To illustrate: If you tell the library to sort rows `[1, 2, 3]` as "asc", it will render them: `3`, `2`, `1`. If you tell it to sort them as "dsc", it will render them: `1`, `2`, `3`.

It's not a huge issue, and may not be worth fixing, since it would break current behavior in all addons (their currently intended sorting order) that rely on this library.

But for reference, here's a page explaining asc and desc order: https://www.mathsteacher.com.au/year7/ch02_power/06_asc/asc.htm

"Numbers are said to be in ascending order when they are arranged from the smallest to the largest number. E.g. 5, 9, 13, 17 and 21 are arranged in ascending order." (in Lib-ScrollingTable that's achieved via "dsc")

"Numbers are said to be in descending order when they are arranged from the largest to the smallest number. E.g. 25, 21, 17, 13 and 9 are arranged in descending order." (in Lib-ScrollingTable that's achieved via "asc")

This is causing me a bit of a mind-fck, since I'm hooking into the "cols" data table to look at the ".sort" property to draw up/down-arrows based on which mode is active. And the misnaming is messing with my head (I have to draw a descending arrow when lib-st is set to ascending, and vice versa...). But it's no huge deal... it's pretty easy to just remember that lib-ScrollingTable's terms are inverted from their intended meaning.

I didn't submit a patch for this one, since it would be a big compatibility change compared to old versions... But if you want to do it, it's here:

			local direction = column.sort or column.defaultsort or "asc";
			if direction:lower() == "asc" then 		
				return a1 > b1;
			else
				return a1 < b1;
			end

Replace with:

			local direction = column.sort or column.defaultsort or "asc";
			if direction:lower() == "asc" then 		
				return a1 < b1;
			else
				return a1 > b1;
			end

 It's unfortunate that such a change would break backwards compatibility... because it's quite an annoying quirk. ;-)

PS: Speaking about optimizations, which is the only real issue with the library, I really don't like the use of `direction:lower() == "asc"`. That function, `lower()`, makes a copy of the string, loops through all characters, and converts them all to lowercase, and then returns the modified string. Now ask ourselves: Why are we doing that on every sort()-callback comparison? If your table has n=500 rows, the sorting algorithm in Lua (quicksort, O(n log(n)) to at worst O(n²) performance), could require up to n^2 (500^2) = 250,000 comparisons (in the worst case) to sort it all in order, which means thousands of pointless string creations/modifications, all because of that `:lower()` usage in the `if direction == "dsc"` function... Two better solutions would be: 1. Don't do that at all. Expect the user to properly write "dsc" instead of something silly like "DSC". Or 2. At least, if this "case insensitive support" is necessary, then just loop through the "cols" array on initial CreateST() call and just :lower() each "defaultsort" value there, a single time, so that it's never uppercase after that.

Personally, I've renamed the lib-st library (to prevent it from conflicting/being overwritten via LibStub), and have removed the `:lower()` from the sort algorithm. (And from the OnClick handler, just for good measure... So I've removed all `:lower()` calls from Core.lua).

This small change, to avoid tens of thousands of wasteful `:lower()` calls, has resulted in column-clicking (which internally re-sorts all data) being noticeably more responsive for the addon user!

PPS: I noticed that the docs pages don't mention how to apply a custom, initial sort-column when the table is created. It may be worth mentioning that it's possible to put "sort" = "asc" (a "sort flag" telling it that we're "sorting by this column") in the cols definition given to CreateST, to cause its initial SortData() call to accept your intended initial sort-column. Otherwise, the code defaults to simply sorting by column 1. If any user does this, they just have to be careful and only put the "sort" flag in ONE of their columns (the way the library's internal code expects it to be).

I'm very sorry for being long-winded... there was a lot to say and I wanted to cover it all. ;-)

In reply to ddumont:

In reply to ddumont:

In reply to ddumont:

The https://www.wowace.com/addons/lib-st/pages/creating-a-scrolling-table/ page example doesnt seem to work. The frame will appear, but immediately bugsack is going

5x lib-st\Core.lua:587: attempt to index field 'cols' (a nil value)<br />lib-st\Core.lua:587: in function `GetCell'<br />lib-st\Core.lua:744: in function `Refresh'<br />lib-st\Core.lua:311: in function `SortData'<br />lib-st\Core.lua:563: in function `SetData'

 There seems to be a need to configure more than the example to get a data filled table, if there is data, if there isnt data if the SetData is {} no such exceptions.

local coldata = {
{ ["name"] = "Person", ["width"] = 113, },
{ ["name"] = "Num1", ["width"] = 113, },
{ ["name"] = "Num2", ["width"] = 55, },
};
local data = {
{ "Sachmo", 12, 4 },
{ "Josua", 8, 2 },
};
local st = LibStub("ScrollingTable");
local table = st:CreateST(coldata);
--table:SetData({});
table:SetData( data );

It looking at some other addons like a registerevents is required?

https://www.wowace.com/addons/lib-st/pages/

table:RegisterEvents{
OnEnter = OnEnter,
OnLeave = OnLeave,
OnClick = OnClick,
OnDoubleClick = OnClick,
}

local function st_OnEnter (rowFrame, cellFrame, data, cols, row, realrow, column, sttable, button, ...)

@ddumont

Alright, just wanted to check. I will see if I can find anything to fix it. Thank you.

In reply to ddumont:

I really like the work you have done on this library. The only problem I have been having is that whenever I open the world map, the tables will permanently disappear. Is it something I could be doing wrong with the code that is causing this?

Is there are way to build heirachy into the table, so for example;

• Cat 1
• Cat 2
• Cat 3
• Cat 4

Then clicking on Cat 2 would then show

• Cat 1
• Cat 2
• Item 1
• Item 2
• Item 3
• Cat 3
• Cat 4

I'd like the Cat rows to be colored differently so they are clearly "Categories"

Upy