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

Remove filter drop down #1227

Merged
merged 5 commits into from
Nov 24, 2021

Conversation

MilanPospisil
Copy link
Contributor

Remove filter drop down when it has only 1 item. It shows text of the selected item instead.

@MilanPospisil
Copy link
Contributor Author

MilanPospisil commented Nov 12, 2021

NOTE - what do you Martin think about the margin solution? I think its not good, but I could not make vertical align with table cell display work :( Though the solution worked in my testing html file, but it is not working in ansible...

My local test file works:

Text! (The HTML does not show until edit mode :) )

@ZitaNemeckova
Copy link
Member

@MilanPospisil Before and After screenshots, please :)

@MilanPospisil
Copy link
Contributor Author

Before
old
:
After
new

@himdel
Copy link
Collaborator

himdel commented Nov 16, 2021

@sbuenafe-rh Do we have any designs for this please?

(Assuming we go with the one in the screenshot...)
AFAICT right now the top padding is too much, the text should align with the "Filter by" text, I think it's off by ~2px (too low).
And we need some sort of right padding, to have some space between the text and the input field, any idea how much?

(by "the text" I mean "Container repository name")

@@ -93,6 +94,14 @@ export class CompoundFilter extends React.Component<IProps, IState> {
isPlain={false}
items={filterOptions}
/>
);
} else {
select = <div style={{ margin: '10px' }}>{filterConfig[0].title}</div>;
Copy link
Collaborator

@himdel himdel Nov 16, 2021

Choose a reason for hiding this comment

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

(I think you'll want something like margin: '8px 4px 0 0', to only set the top and right one (+- numbers, or possibly +-margin vs padding))

But if it helps, .pf-c-input-group is a flex element, it might help with the centering, if centering is enough.

@sbuenafe-rh
Copy link

@himdel it should only be the Filter by container repository name input field. Container repository name text in the front of this field should be removed.

@MilanPospisil
Copy link
Contributor Author

@himdel it should only be the Filter by container repository name input field. Container repository name text in the front of this field should be removed.

So the name should not be visible at all?

@himdel
Copy link
Collaborator

himdel commented Nov 23, 2021

So the name should not be visible at all?

That's right, the user can still see it in the placeholder or in active filters.

20211123202350 20211123202338

20211123202619
20211123202627

LGTM 👍

@MilanPospisil MilanPospisil merged commit c6d04fc into ansible:master Nov 24, 2021
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.

None yet

4 participants