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

[Signing 2] Add sign badge to collection card #1628

Closed
wants to merge 1 commit into from
Closed

[Signing 2] Add sign badge to collection card #1628

wants to merge 1 commit into from

Conversation

brumik
Copy link
Contributor

@brumik brumik commented Feb 9, 2022

Screenshot from 2022-02-09 10-38-21
Screenshot from 2022-02-09 10-37-38

@brumik brumik requested a review from himdel February 9, 2022 09:40
@brumik brumik mentioned this pull request Feb 9, 2022
2 tasks
@brumik
Copy link
Contributor Author

brumik commented Feb 9, 2022

If this PR is fine, I'll update the tests (they rely on a selector which is not there anymore)

@brumik brumik changed the title [Signing 2] Add sign badge to collection card - refactor [Signing 2] Add sign badge to collection card [WAIT FOR API] Mar 2, 2022
@ZitaNemeckova
Copy link
Member

ZitaNemeckova commented Mar 3, 2022

@brumik There are two modes to display collections and in the second one (a list one) there's no sign label. Is that on purpose?

(The icons to change the view are to the left of the top pagination.)

Screenshot 2022-03-03 at 10 52 29

@brumik
Copy link
Contributor Author

brumik commented Mar 3, 2022

@ZitaNemeckova I did not realized it, I think it should have some sign notifications there too. @trahman73 any mockups for this or can I go as I feel would be right following the conventions of the namespace page?

@trahman73
Copy link

@brumik Good catch, we can follow the pattern from the Namespaces area here. Thanks!

@brumik brumik marked this pull request as draft March 3, 2022 14:08
@brumik brumik changed the title [Signing 2] Add sign badge to collection card [WAIT FOR API] [Signing 2] Add sign badge to collection card Mar 3, 2022
@brumik
Copy link
Contributor Author

brumik commented Mar 3, 2022

@ZitaNemeckova In the Namespaces PR here:
#1629 -- the namespace list and the collection list uses the same component for displaying a single collection, so this is the part of the other PR.

@brumik brumik marked this pull request as ready for review March 3, 2022 14:31
@himdel
Copy link
Collaborator

himdel commented Mar 14, 2022

here before
20220314170753 20220314170929
20220314170812 20220314170920
20220314170824 20220314170850

@himdel
Copy link
Collaborator

himdel commented Mar 14, 2022

^ Seems like we get inconsitent card sizing now, and title and company don't get cut off with ellipsis.

Is that something you can fix, or should we just add the badge to the existing cards without changing how they work?

himdel added a commit to himdel/ansible-hub-ui that referenced this pull request Mar 14, 2022
replaces ansible#1628

this just adds the signature badge to collection card
@himdel
Copy link
Collaborator

himdel commented Mar 14, 2022

I think we can replace this by #1763 , looks like adding SignatureBadge to the existing CardHeader works fine by itself :)

himdel added a commit to himdel/ansible-hub-ui that referenced this pull request Mar 14, 2022
replaces ansible#1628

this just adds the signature badge to collection card
@brumik
Copy link
Contributor Author

brumik commented Mar 15, 2022

I probably just missed a PF prop on the card. But your PR seems smaller, we can roll with that.

Closing in favour of #1763

@brumik brumik closed this Mar 15, 2022
himdel added a commit that referenced this pull request Mar 24, 2022
* CollectionCard: add SignatureBadge

replaces #1628

this just adds the signature badge to collection card

* ComponentCard - add left margin for signature badge

useful when a logo is too wide

No-Issue
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