Add story filter feature

[mateusdeap]

Description:

This PR adds a search bar at the top of the stories table that allows us to filter the stories by their names, using a debounce function @fbuys graciously taught me to implement.

I believe there might be some usability questions so I'd be grateful for some feedback in that area.

So, before, we had this:

And here's the search bar:

Should this be merged, it closes #49

---

I will abide by the code of conduct.

[mateusdeap]

So the build is failing when it runs bundle update rails for the next rails version. However, I tried running the same command locally and it worked fine. On monday I'll try to debug what might be going on.

[kindoflew]

hey @mateusdeap! should we add WIP to the title in the meantime or do you think it'll be a quick fix? thanks!

[mateusdeap]

I believe you're right @kindoflew. I'm not really sure whether this is a quickfix or not.

[fbuys]

It seems like we should replace app/views/projects/_project_table.html.erb with app/views/projects/_stories_table.html.erb?

Could we potentially hide the search bar while it is not being used?
Perhaps similar to how the export settings are hidden.

I tested it locally and it seems to work really well. 🚀

[fbuys]

Do you think calling it query would add any clarity?

Suggested change

	@q = @project.stories.by_position.includes(:estimates).ransack(params[:q])
	@query = @project.stories.by_position.includes(:estimates).ransack(params[:q])

[arielj]

I'd say to call it stories as it was before, since it's querying stories, q or query are too generic

[mateusdeap]

In this case might be better to call it stories, but I have to add a call to .result at the end. Or we can just change it to @query, since I assign @stories in the line below

[fbuys]

I agree, query.result reads better than stories.result And then you assign to @stories like you said.

[fbuys]

What does :title_cont stand for?

[mateusdeap]

That's how ransack matches fields. Here they explain what suffixes we can add. In this case it just means title contains

[fbuys]

I see.

I think it would be better to map something like search_term to title_cont in the backend.
I might be wrong, but it seems like we are putting some implementation details that don't belong in a view here.

I think a future developer might not understand what _cont stands for and making it more explicit could be useful.

[fbuys]

Just read Ariel's response, so we can ignore my reply above :-)

[fbuys]

I think we should try to add local: false?

Suggested change

	<%= search_form_for @q, url: project_path(id: @project.id) do |f| %>
	<%= search_form_for @q, url: project_path(id: @project.id), local: false do |f| %>

Right now I can hit enter/return and it reloads the page.

[mateusdeap]

Definitely

[arielj]

Is this only searching on the title? if so maybe we can have an implementation to filter the table in place with javascript?

since we have no pagination or anything, filter in place should be simple (go over all the rows and hide any row that doesn't match the search term) and then we don't need ransack or an ajax request

if we keep the server filtering, do we need ransack for this? I'd say to just use a simple where("title LIKE :q OR description LIKE :q", "%#{params[:q]}%") (it can be a named scope in Story also), so we don't have an extra gem for something like this, ransack is more useful when we need to do comples searches with variable fields and such, but I think it's overkill for this

[mateusdeap]

@arielj Thanks for the suggestion. I'll go for an implementation then. I just used ransack because I was used to it from the past hahaha

@fbuys Only now I noticed that both files were kept. I wanted to have only a projects_table since that was more in line with all the classes in the divs and whatnot. But, originally, I had made it a stories_table because that's what it was. What would be your take on this?

[mateusdeap]

Still pending advice on the UX for this. @arielj already shared his thoughts, but I want to wait for a bit more. Fixed many issues and the filtering is now done in the frontend.

[arielj]

Looks good to me, I think the input should be aligned to the right instead of using the full width of the screen

I'm looking at this design

though that would break a feature (having the add story button always visible) unless we make that part also sticky like the table header which feels weird to me

I'd say to just move the input to the right side (but above the add story / delete buttons, not inline)

[mateusdeap]

@arielj It would? Is it not possible to implement that design without hiding the add_story button? Not sure I got that part. But I completely forgot about this design discussion hahaha Will work on it in the afternoon

[natsubau]

@arielj, I think adding the buttons and search to the sticky area would be fine, not weird. I think it would just be different to what has been there for a while. Is there a specific reason you think it would be weird?

[arielj]

Sorry, what I mean is that we already have the table header sticky at the top and we were using that space to keep the Add story button always visible. If we move the Add story button above the header we'll have to make 2 things sticky: the search+button and the table header. I imagine a lot of empty spaces used by both sticky elements (empty space at the left of the search, empty space on the last column header). Or do you imagine the search+button and the table header overlapping in the same space? or something different? maybe I'm just imagining something different, an example of the scrolled design with how the sticky elements look would be great (though don't consider this is a priority or anything over other projects).

Anyway, doing that change involves extra css and html changes, for 2 sticky elements to work together we'll have to change some things that I think may be outside of the scope of this story, we can handle the position of the search field in this one and moving the buttons in another story too.

Or maybe the Add story button should be in a different place completely. We wanted it to be always visible because it's the most used action.

I think it's a good idea to rethink the position of everything in this app though. Maybe it's something to plan over the year?

[arielj]

this looks good to me

[lubc]

@kaysiz hey could you please QA this PR? Thank you!

[kaysiz]

Looks good!