Add an filter/search box for the app list page #90
Conversation
Fixes mesosphere/marathon#1204. I tried to stick to the design mockups on that page.
|
Hi @dfuentes, thanks for this PR! I've tested it thoroughly and it's working nicely. I noticed you included the Ionicons set. So far we've got by without using any icons at all, so this is quite a big code change. I thought Glyphicons might be preferable, as the styles are already in courtesy of bootstrap, or maybe we should simply include the two icons as image assets. @aatumanova, may we ask your opinion here? |
| </Link> | ||
| <AppListComponent /> | ||
| className="btn btn-success" | ||
| style={{float: "right"}}> |
There was a problem hiding this comment.
Let's use the bootstrap pull-right class here instead of a style attribute.
|
I mainly used those icons based on the comment from @aatumanova here, but would be happy to change them. |
| <TabPaneComponent id={tabs[0].id}> | ||
| <Link | ||
| <div className="row"> |
There was a problem hiding this comment.
These columns don't collapse nicely on narrow screens - let's remove them. The button is already floated right, and the filter component can be prevented from filling the screen with max-width: 330px or similar.
|
@dfuentes re the icon set - thanks, I hadn't seen that comment. Ionicons it is. |
|
@philipnrmn There have actually been a discussion around which icon set to use, and we landed on Ionicons. I think the plan is to add a custom icon set when canvas is introduced to the project, but I think this is a good start. |
|
Oh, didn't see your comment @philipnrmn |
|
Updated based on feedback:
|
| "input-group": true, | ||
| "filter-box": true, | ||
| "filter-box-activated": !!state.activated, | ||
| "pull-left": true |
There was a problem hiding this comment.
I don't think the filter input needs to float left - floating the button right ought to be enough.
There was a problem hiding this comment.
That's what I thought as well, however with only the button floated right, they no longer align vertically. This is because bootstrap uses display: table for input groups. Not using input groups would be a larger change (since thats how I implemented the icons inside the input).
I'm not great at css, so other suggestions are definitely welcome.
There was a problem hiding this comment.
Ah, good point - I made an omission in my feedback. The button should precede the filter input in the markup to prevent it wrapping under.
- remove float: left from filter box - reorder filter box and nav button so they align correctly - use max-width instead of width for filter box
|
Thanks, reordering did let me just float the button. Made that change as well as the max-width one. |
|
Looks fantastic. I'm happy to merge, assigning to @pierlo-upitup for a final review. |
|
Excellent job, @dfuentes – thanks! |
Add an filter/search box for the app list page
|
Lovely work @dfuentes! |
|
Cool!! |
This PR fixes mesosphere/marathon#1204 by adding an app search/filter box. I tried to stick to the design mockup that is on that issue's discussion.
Can see a small demo of the functionality here