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

webui: provide an About screen #4849

Merged
merged 1 commit into from
Jul 20, 2023
Merged

Conversation

acruzgon
Copy link
Contributor

@acruzgon acruzgon commented Jun 21, 2023

INSTALLER-3489

Provide an About screen for Anaconda Web UI using AboutModal PF component.
image

@acruzgon
Copy link
Contributor Author

@garrett here's the initial draft, let me know if the margins and size look right. thank you!

Copy link

@garrett garrett left a comment

Choose a reason for hiding this comment

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

Hi! This is a good start of the scaffolding needed to add the global menu and about box. I'll need more work before it can be merged, but I think you know this. 😁

  • The logo and name are hardcoded. They shouldn't be. Each distribution needs their own SVG logo, which should also be used in the header in addition to the about box.

  • The logo is not positioned correctly. It should use PatternFly's positioning; it should not be hardcoded with margins. The end result should be vertically centered with the × (close) icon of the about modal.

  • The subheading needs to be an H2, not an H1.

  • Package names and versions should be generated.

  • We'll need branding support, so each distribution will have custom settings with CSS Custom Properties (CSS variables) in a CSS file, and those variables should be used here.

  • The × is too small. The mockups were made with PatternFly 5 in mind, which uses this larger × without the circle, whereas PatternFly 4 uses a circle around the icon to indicate the button area. We'll either need to enlarge the × or change the background of the circle around it, so it looks clickable.

Related:

ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/HeaderKebab.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/app.scss Outdated Show resolved Hide resolved
ui/webui/src/components/app.scss Outdated Show resolved Hide resolved
@garrett
Copy link

garrett commented Jun 22, 2023

Fedora:

About modal, no bg, Fedora

CentOS:

About modal, no bg, CentOS

RHEL:

About modal, no bg, RHEL

Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

I would suggest to get this PR in first without the logo - as this will need some more work.

ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
@KKoukiou KKoukiou marked this pull request as draft June 23, 2023 11:46
@garrett
Copy link

garrett commented Jun 27, 2023

I would suggest to get this PR in first without the logo - as this will need some more work.

This is why I suggested using the generic icon originally.

About modal(1)


But we can drop all that stuff and make some overrides:

/* Scale down the about box to be a little smaller */
@media only screen and (min-width: 992px) {
  .pf-c-about-modal-box {
    --pf-c-about-modal-box--lg--MaxWidth: 55rem;
    --pf-c-about-modal-box--lg--MaxHeight: 32rem;
    grid-template-rows: 3rem max-content auto;
  }
}

/* Hide the logo and side graphic */
.pf-c-about-modal-box__brand,
.pf-c-about-modal-box__hero {
    display: none;
}

/* Make the close button more obvious, since we hide the side graphic*/
.pf-c-about-modal-box__close .pf-c-button.pf-m-plain {
  --pf-c-about-modal-box__close--c-button--BackgroundColor: var(--pf-global--BackgroundColor--dark-200);
}
.pf-c-about-modal-box__close .pf-c-button.pf-m-plain:hover
  --pf-c-about-modal-box__close--c-button--BackgroundColor: var(--pf-global--BackgroundColor--dark-400);
}

This should make it look like the following:

About modal, no bg, no branding

@acruzgon
Copy link
Contributor Author

I would suggest to get this PR in first without the logo - as this will need some more work.

This is why I suggested using the generic icon originally.

About modal(1)

But we can drop all that stuff and make some overrides:

/* Scale down the about box to be a little smaller */
@media only screen and (min-width: 992px) {
  .pf-c-about-modal-box {
    --pf-c-about-modal-box--lg--MaxWidth: 55rem;
    --pf-c-about-modal-box--lg--MaxHeight: 32rem;
    grid-template-rows: 3rem max-content auto;
  }
}

/* Hide the logo and side graphic */
.pf-c-about-modal-box__brand,
.pf-c-about-modal-box__hero {
    display: none;
}

/* Make the close button more obvious, since we hide the side graphic*/
.pf-c-about-modal-box__close .pf-c-button.pf-m-plain {
  --pf-c-about-modal-box__close--c-button--BackgroundColor: var(--pf-global--BackgroundColor--dark-200);
}
.pf-c-about-modal-box__close .pf-c-button.pf-m-plain:hover
  --pf-c-about-modal-box__close--c-button--BackgroundColor: var(--pf-global--BackgroundColor--dark-400);
}

This should make it look like the following:

About modal, no bg, no branding


with that it would like this:
image
if i add display: block for the title it would look like this:
image

@garrett what do you think?

Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

Pls complete the translations, rebase and adjust the rest of the comments but this will need more review rounds.

ui/webui/src/helpers/product.js Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
@garrett
Copy link

garrett commented Jul 3, 2023

Perhaps it needs to be a little wider. I didn't account for the parenthesis. It could be longer in other languages (if
prerelease" is translated), and we don't want it to get truncated.

For example:

About modal, no bg, Fedora(3)

To get it this width and height, change the CSS a little, like so:

    --pf-c-about-modal-box--lg--MaxWidth: 62rem;
    --pf-c-about-modal-box--lg--MaxHeight: 38rem;

@acruzgon acruzgon force-pushed the INSTALLER-3489 branch 4 times, most recently from 911c1c1 to 1e75ed4 Compare July 12, 2023 18:28
@acruzgon acruzgon marked this pull request as ready for review July 12, 2023 18:29
ui/webui/src/components/aboutmodal.scss Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/aboutmodal.scss Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/helpers/product.js Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

The code is pretty close but we need tests.

ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/index.js Outdated Show resolved Hide resolved
Copy link
Contributor

@KKoukiou KKoukiou left a comment

Choose a reason for hiding this comment

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

Still missing tests. Can help you with that today.

ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/AnacondaAboutModal.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/HeaderKebab.jsx Show resolved Hide resolved
ui/webui/src/components/HeaderKebab.jsx Outdated Show resolved Hide resolved
ui/webui/src/components/HeaderKebab.jsx Outdated Show resolved Hide resolved
@KKoukiou
Copy link
Contributor

/kickstart-test --waive webui only

@KKoukiou
Copy link
Contributor

/kickstart-test --waive webui only

@KKoukiou KKoukiou merged commit 99cd49a into rhinstaller:master Jul 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

5 participants