canonical / charmhub.io

GNU General Public License v3.0
10 stars 22 forks source link

fix: add metadata to side-navigation on charm details page #1843

Open chillkang opened 3 months ago

chillkang commented 3 months ago

Done

How to QA

Testing

Issue / Card

Fixes https://warthogs.atlassian.net/browse/WD-10007 Fixes https://warthogs.atlassian.net/browse/WD-10933 Fixes https://warthogs.atlassian.net/browse/WD-10934 Fixes https://warthogs.atlassian.net/browse/WD-10935 Fixes https://warthogs.atlassian.net/browse/WD-10936 Fixes https://warthogs.atlassian.net/browse/WD-10937

Screenshots

Screenshot 2024-06-20 at 2 34 36 PM
webteam-app commented 3 months ago

Demo

Jenkins

demos.haus

lukasSerelis commented 3 months ago

Looks good, but specific example you've given here shows an issues with this. The product site is meant to be used for charms that have product pages outside of charmhub, like mySQL. (Product page for MySQL).

In most cases website is just the link to charm, which is useful in github docs, but less useful as a link within charmhub. If possible, if the product link could only be shown if it's not a link to charm in charmhub would be ideal. Not sure how possible, but would be much preferred.

Other than that all look really good!

Lukewh commented 3 months ago

I'm going to say the same thing. From what I understand the "product page" is meant to be a single link. But for https://charmhub-io-1843.demos.haus/mysql for example, there's a list with no way to discern what each link will do. I'm not sure how the design handles multiple links, or how we would differentiate between the upstream product page and a general link? In snapcraft we split it up into different link types: https://snapcraft.io/lukewh-test

lukasSerelis commented 3 months ago

We could have multiple solutions for this.

  1. Either just display links that aren't labeled the way snapcraft does it (just show the domain and add to other links section)
  2. Or force a stricter structure on metadata.yaml/charmcraft.yaml where each link if named (like Charm repository, charm product page, etc.) has to be written in the correct format to be displayed, otherwise it won't be shown
  3. 2nd option but add other links section for every link that isn't following the preset links.

Happy to discuss, but need to make a decision on this before moving forwards

chillkang commented 3 months ago

We could have multiple solutions for this.

  1. Either just display links that aren't labeled the way snapcraft does it (just show the domain and add to other links section)
  2. Or force a stricter structure on metadata.yaml/charmcraft.yaml where each link if named (like Charm repository, charm product page, etc.) has to be written in the correct format to be displayed, otherwise it won't be shown
  3. 2nd option but add other links section for every link that isn't following the preset links.

Happy to discuss, but need to make a decision on this before moving forwards

@lukasSerelis I'm happy with either option and let's discuss this next Monday :)

lukasSerelis commented 3 months ago

We've discussed how to deal with the multi link situation and this is where we've left it:

lukasSerelis commented 3 months ago

There's a small UI issue here as well, where the space between icon and text isn't margin or padding, it's just to spaces. This is the spacing that should be followed image

lukasSerelis commented 3 months ago

For whatever reason pgbouncer charm has a product link as first link in websites: also has a source: but show the github link as the product page link and no source link at all

https://charmhub-io-1843.demos.haus/pgbouncer image

chillkang commented 3 months ago

For whatever reason pgbouncer charm has a product link as first link in websites: also has a source: but show the github link as the product page link and no source link at all

https://charmhub-io-1843.demos.haus/pgbouncer image

You found the outlier! This is the metadata I'm getting so I'll dig into this issue.

lukasSerelis commented 3 months ago

Another one that follows the same mistake: https://charmhub-io-1843.demos.haus/kafka-k8s