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

Interactive table sorting, trimming, and filtering #152

Merged
merged 18 commits into from
Aug 11, 2022

Conversation

digocorbellini
Copy link
Contributor

Issue #, if available:

Description of changes:
Added three new features to the bubble tea table. They can be used both individually and at the same time. These changes are intended to make it easier to compare instance types without having to re-run instance selector.

1) Filtering rows by strings.

filtering_example.mov

Only rows containing the passed in string will be visible. Exists in tableView.go

2) Trimming rows to only selected rows

trimming_example.mov

Trim the visible rows to only show selected rows. Exists in tableView.go

3) Sorting

Sorting_Example.mov

Sort instance types using the same 2 methods available to CLI users: using shorthand flags (like memory, spot-price, etc.) or using a json path. Exists in sortingView.go

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@digocorbellini digocorbellini self-assigned this Aug 6, 2022
@digocorbellini digocorbellini requested a review from a team as a code owner August 6, 2022 02:53
pkg/selector/outputs/bubbletea.go Outdated Show resolved Hide resolved
pkg/selector/outputs/verboseView.go Outdated Show resolved Hide resolved
@@ -62,30 +76,92 @@ func (m BubbleTeaModel) Init() tea.Cmd {
func (m BubbleTeaModel) Update(msg tea.Msg) (tea.Model, tea.Cmd) {
switch msg := msg.(type) {
case tea.KeyMsg:
// don't listen for input if currently typing into text field
if m.tableModel.filterTextInput.Focused() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about moving this table- and sorting-related logic to the respective view files, and then forwarding the key msg to the Update() of tableView and sortingView? Similar to the other "in focus" handling you do there.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These if statements exist to prevent the keys used to switch between states from causing a state switch while typing in a text field.

case "enter":
// sort and switch states to table
if m.currentState == stateSorting {
sortFilter := string(m.sortingModel.shorthandList.SelectedItem().(item))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Similarly here, once we know we're in the sortingView state, we should pass the key msg to sortingView's update() and etc. to group all of the actual sorting logic together.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For the sorting, information from the sorting state must be retrieved and then passed on to the table state before the state switch occurs. This requires access to both sortingModel and tableModel which are not currently reachable within the update() functions of each view.

I tried to keep all logic which dealt with "inter-state" affairs (such as switching states) in the Update() found in bubbletea.go, which is why the sorting logic currently lies there.

pkg/selector/outputs/sortingView.go Outdated Show resolved Hide resolved
@AustinSiu
Copy link
Contributor

The only reference to interactive mode in the README is kind of buried in the output flag's --help description.
Could you help make the fact that we have an interactive mode more visible? e.g. ec2-spot-interrupter's docs

@bwagner5
Copy link
Contributor

bwagner5 commented Aug 11, 2022

A short video demoing the features like you have in the PR (but combined and maybe 15-20 seconds) would be really cool on the readme!

@digocorbellini
Copy link
Contributor Author

@bwagner5 @AustinSiu Added a demo video to the readme under the CLI examples. Change found here: 606ca51

Copy link
Contributor

@AustinSiu AustinSiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks Rodrigo, this will definitely come in handy 👍

@digocorbellini digocorbellini merged commit adc1627 into aws:main Aug 11, 2022
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