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

Add SVG topology to bundles #775

Merged
merged 1 commit into from Jan 20, 2021

Conversation

solazio
Copy link
Contributor

@solazio solazio commented Jan 12, 2021

Done

  • Add SVG topology to bundles oveview page.

How to QA

Issue / Card

Fixes #

Screenshots

@webteam-app
Copy link

Demo starting at https://charmhub-io-775.demos.haus

Copy link
Contributor

@anthonydillon anthonydillon left a comment

Choose a reason for hiding this comment

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

Why not just add the topology to the template above the readme? It will give you more freedom to mark it up and its where the next developer will first look to find it.

Does the design ask for it to be injected into the readme?

@solazio
Copy link
Contributor Author

solazio commented Jan 12, 2021

Why not just add the topology to the template above the readme? It will give you more freedom to mark it up and its where the next developer will first look to find it.

Does the design ask for it to be injected into the readme?

@danielmutis said that he wants the title to come first as per the design. @danielmutis what do you think?

@tbille
Copy link
Contributor

tbille commented Jan 12, 2021

The main title of the page is the bundle name on the header.

As a developer I would expect to see my readme not modified with injected image. It seems more natural to have before the readme to get a quick understanding of the bundle.

It also removes a code complexity, and we like simple code 😆

@danielmutis
Copy link

danielmutis commented Jan 13, 2021

@solazio Let's push the whole readme below the image then.

@tbille @anthonydillon my main worry about the image is that it pushes the content down by a lot because they are quite large. I wonder if maybe we should hide part of it and have a show more.

@anthonydillon
Copy link
Contributor

You could put the topology in its own tab?

Copy link
Contributor

@anthonydillon anthonydillon left a comment

Choose a reason for hiding this comment

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

Thanks @solazio, few comments inline.

static/js/src/public/details/index.js Outdated Show resolved Hide resolved
templates/details/overview.html Outdated Show resolved Hide resolved
static/js/src/public/details/index.js Show resolved Hide resolved
templates/details/overview.html Outdated Show resolved Hide resolved
templates/details/overview.html Outdated Show resolved Hide resolved
Copy link
Contributor

@anthonydillon anthonydillon left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@anthonydillon anthonydillon self-assigned this Jan 19, 2021
@solazio solazio merged commit 4822eb2 into canonical:add-bundles Jan 20, 2021
solazio added a commit that referenced this pull request Jan 20, 2021
tbille pushed a commit that referenced this pull request Jan 20, 2021
* Make bundles visible on homepage (#776)

* Add SVG topology to bundles (#775)

* Fix details page bundle icon

* Show only the overview tab for bundles
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants