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

feat: custom filter text #116

Closed
lrstanley opened this issue Jul 4, 2022 · 13 comments
Closed

feat: custom filter text #116

lrstanley opened this issue Jul 4, 2022 · 13 comments
Assignees
Labels
enhancement New feature or request

Comments

@lrstanley
Copy link

lrstanley commented Jul 4, 2022

Hello again -- as shown in #115 as an example, my app layout includes a "command bar" that can both invoke commands and filter views. I plan to support a bunch of functionality within this input that is not just a plain search string (e.g. the ability to also use key=value or !shebang type entries). Due to this, directly exposing it to bubble-table won't work, as the Value() output won't be what I'm looking to filter with. The way my usecase works, is it sends a message to all components through the app, with the filter text when it changes, allowing each component to individually filter based off the specific usecases of each component.

What would be awesome to have supported, is a field that lets me specify the actual text for filtering, where "" is no filtering, and anything else would be used as filtering. With this, I would be able to support key-value type filtering, but also regular text filtering.

Alternatively, loosening the restrictions on the WithFilterInput() method, to utilize an interface of Value() string, and selectively interface checking for other features like Focus(), so it only calls those if the underlying type has those methods.

Thoughts?

@Evertras Evertras added the enhancement New feature or request label Jul 4, 2022
@Evertras
Copy link
Owner

Evertras commented Jul 4, 2022

Agree this is a useful/reasonable feature, and I think the interface approach might be the nicest for all involved. That would allow you to inject whatever behavior/checks you want and just make sure it's spit out with Value(). Let me give that a shot.

@Evertras
Copy link
Owner

Evertras commented Jul 4, 2022

I lied, the interface approach would not have been the nicest for the table itself. There are a lot of additional checks that are done that make this unfortunately difficult. Fortunately a very easy fix is to just expose the set function, which is what #117 does.

Released as v0.14.3, let me know if that works for you. If it does, feel free to close the issue. If you think we need to do more, let's keep this open and discuss.

@lrstanley
Copy link
Author

Testing the new functionality out, it looks like it is receiving the filter input, however it's not actually filtering. Is something additional required to get it to actually filter once the value is specified?

@Evertras
Copy link
Owner

Evertras commented Jul 5, 2022

Hmm, did you use .Filtered(true) with the table? You'll still need to specify that to use any filtered value provided.

@Evertras
Copy link
Owner

Evertras commented Jul 5, 2022

Though that is generally some strange behavior that you're showing in that gif regardless. Even if .Filtered fixes your issue I'd like to keep this open until I fix that appropriately.

@lrstanley
Copy link
Author

Yep! .Filtered(true) and .Focused(true) is set.

@Evertras
Copy link
Owner

Evertras commented Jul 5, 2022

Got it, I'll have to take a look when I get some time. Sorry about that, I was relying too much on the added test rather than trying to construct a whole example and clearly something got mixed up somewhere.

@lrstanley
Copy link
Author

Ok, the filtering part was my fault. I didn't realize I had to set .WithFiltered(true) on the columns in question, in addition to the model-level filters. With that, filtering does work as expected.

I do still see the odd coloring at the bottom -- not sure if that's an issue on my end or not:

Just made the source public: https://github.com/lrstanley/hangar-ui/blob/master/internal/ui/view/targets.go#L42-L52

@Evertras
Copy link
Owner

Evertras commented Jul 5, 2022

Ah, that makes sense! Glad it's working.

For the color, it appears to be tied to the cursor of the filter. I'm able to reproduce color reset when the cursor blinks in and out. I think the cursor itself is resetting the style somehow, I'll investigate a bit more.

@Evertras
Copy link
Owner

Evertras commented Jul 5, 2022

Turns out the cursor is accidentally injected when setting the text input value directly, so it needs to be reblurred. This should get rid of the block cursor for you and fix your issue, though there's a lingering issue that I'm going to make another github issue for as a separate problem.

Released as v0.14.4, try that and see if the color issue is fixed for you. Additionally, you may want to use a static footer to get rid of the filter text appearing, but up to you - I can see it being useful either way.

@lrstanley
Copy link
Author

lrstanley commented Jul 5, 2022

I actually set a static footer, but it seems like it doesn't apply in some circumstances -- will test that more later today. I think it's because the way I currently have it setup, sometimes it's empty, which makes the default one render. If I add pagination into it, it'll probably override and continue to be used.

Will test out the fixes later today and will let you know. Thanks!

@lrstanley
Copy link
Author

lrstanley commented Jul 6, 2022

Issues have been fixed. I also noticed that GetVisibleRows() returns all rows, not just the visible ones (i.e. the number that are visible on the current page, and if on the last page, that could theoretically be less than the page size). Is this expected behavior? I would think not, given TotalRows() exists, but wasn't sure.

@Evertras
Copy link
Owner

Evertras commented Jul 9, 2022

Yes that's some unfortunate naming that slipped in early on. It's intended for all rows that are actually included in the table's view (filtered/sorted), and paging is applied on top of that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants