Added button to toggle visibility of disabled agents #1464
Conversation
|
Cool idea @International! I wonder if we should do this server-side with filtering on the list of Agents returned from the database? We could use a cookie to store the user's preferences. |
|
Sure, we could do it that way also. Would it then make sense for the button to be a button? Or should it be a checkbox? Also, do you think it would make sense to move this menu bar to the top of the table, instead of having the users scroll through a potential long list? |
|
Maybe it could be a small checkbox in the header of the table in the |
|
I'm not that great with css or stuff related to design ... did you mean something like this? Does not look all that great imho :) |
|
Hmm, agreed. I guess let's go for a button for now, like your original design. It should probably still set a cookie on the backend and filter in Arel instead of JS, though, so that pages are all the same size when rendered. |
|
@cantino can you have a look at the PR? |
|
|
||
| private | ||
| def show_only_enabled_agents? | ||
| !!cookies[:huginn_view_only_enabled_agents] |
There was a problem hiding this comment.
Would it make sense to store that in the session instead of separate cookie?
There was a problem hiding this comment.
either solution works from my point of view
|
I like the idea! It would be great to have the same functionally for the |
| @@ -0,0 +1,10 @@ | |||
| class @AgentIndexPage | |||
There was a problem hiding this comment.
You can remove this JS now, right?
There was a problem hiding this comment.
i've removed it
|
Nice! |
@dsander not sure how much time I will have the next 2 weeks or so, but if I get some moments, I'll see what I can do :) |
|
The spec failure looks unrelated. Is this ready to merge @International? |
|
Yes, it is. I've ran all the tests locally multiple times, they all pass. |
| @@ -6,6 +6,14 @@ def agent_show_view(agent) | |||
| end | |||
| end | |||
|
|
|||
| def toggle_disabled_text | |||
| if cookies[:huginn_view_only_enabled_agents] | |||
| "Show Disabled Agents" | |||
There was a problem hiding this comment.
One minor thing, can you add a leading space like it is done for the other links? The text looks a little bit cramped right next to the icon.
|
@International Just some minor nitpicking, looks good to me otherwise! |
|
@dsander thanks, it's done! |
|
Nice, thanks you. Do you mind me squashing the commit during the merge or would you like to do it manually? |
|
No problem, you can squash it . |
|
Great, thanks for working on this! |
It happens that over time I disable some agents, but keep them in the list, for possible future use. However, they tend to take some space. With this PR, I've added a button to the bottom list, to be able to toggle visibility of disabled agents. Do you think this would be an useful addition to the project?
Screenshot:
