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

Add methods to enable live updating of Tables #12

Merged
merged 3 commits into from
Oct 21, 2024

Conversation

drmille2
Copy link
Contributor

add UpdateByAsc and UpdateByDesc methods
add GetOrder method

these methods provide the ability to clear the contents of an existing table so that new data can be added prior to re-rendering, enabling live updating tables

example usage:

    m.table.ClearRows()
    var rows [][]string
    for _, m := range msg {
        rows = append(rows, toRow(&m))
    }
    m.table.AddRows(rows)
    if m.orderPhase == 0 {
        m.table.OrderByAsc(m.orderColumn)
    } else {
        m.table.OrderByDesc(m.orderColumn)
    }

add UpdateByAsc and UpdateByDesc methods
add GetOrder method

these methods provide the ability to clear the
contents of an existing table so that new data
can be added prior to re-rendering, enabling
live updating tables
@drmille2 drmille2 changed the title add the ClearRows method Add methods to enable live updating of Tables Mar 25, 2023
@drmille2
Copy link
Contributor Author

A few minor additive changes in this PR. Was trying your very nice Table module out with the intent of using it as a live updating view for a gRPC service, but I was not able to find any way in the implementation to update rows after adding them (either updating their content, or removing rows that had expired).

I started by adding a ClearRows() method, but found that just clearing the rows and re-adding a new set during the model Update() resulted in any ordering being lost, and just persisting & re-executing OrderByColumn() would toggle between ascending and descending. Adding in the OrderByAsc/Desc provided a deterministic way of re-applying any previous ordering that can now be retrieved with GetOrder().

Works pretty well in practice, happy for any feedback or suggestions.

@76creates
Copy link
Owner

76creates commented Apr 9, 2023

Hey there @drmille2, thanks for taking time to contribute to the stickers!
I agree with the changes for sure, toggling is not really deterministic and requires potentially 2 renders to get you to where you want to be.
It would be good to remove OrderByColumn at some point, but for backwards compatibility we could keep it for now. Please add Depricated comment box to the function so IDE's can warn users about this.

[Update]
Please update examples that use sorting.

Sorry for the lag, I had no time to commit to stickers past few months, still is bit hectic but I will try to be around more :)

@76creates
Copy link
Owner

@jon4hz there were lots of breaking updates on #10, packages were moved to separate directories, can you please update the PR?

@drmille2
Copy link
Contributor Author

drmille2 commented Apr 13, 2023

Went ahead and merged in the latest changes from master, so the PR should be compatible with the breaking changes from #10 now, and added a deprecation warning to OrderByColumn.

@76creates
Copy link
Owner

Went ahead and merged in the latest changes from master, so the PR should be compatible with the breaking changes from #10 now, and added a deprecation warning to OrderByColumn.

Not sure how I missed this one, sorry for lag, will check it out soon!

@drmille2
Copy link
Contributor Author

Any updates on this--concerns or other things you'd like to see addressed?

@spogatetz
Copy link

@76creates Any chance this can get merged? Or are there any workarounds I can use? I tried recreating the table but it doesn't seem to be working.

@76creates 76creates merged commit 3d97911 into 76creates:master Oct 21, 2024
@76creates
Copy link
Owner

@spogatetz checked it again, all legit, merged ⚡

@76creates
Copy link
Owner

Released with 1.4.1

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

Successfully merging this pull request may close these issues.

3 participants