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.

Sign up for GitHub

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

Jump to bottom

[RHOAIENG-5491] Landing page: Info section about AI flows #2723

Merged
merged 1 commit into from
Apr 22, 2024
Merged

[RHOAIENG-5491] Landing page: Info section about AI flows #2723

merged 1 commit into from
Apr 22, 2024

Conversation

jeff-phillips-18
Copy link
Contributor

@jeff-phillips-18 jeff-phillips-18 commented Apr 18, 2024 •
edited
Loading

Closes: RHOAIENG-5491

Description

Show an information section about AI flows in the landing page.

Screen shots

image

image

image

image

Projects disabled

image

Projects and Model serving disabled

image

Pipelines, Model serving, and projects disabled

image

How Has This Been Tested?

Tested manually by turning flags on/off.

Test Impact

Added e2e tests.

Request review criteria:

  • The developer has manually tested the changes and verified that the changes work
  • Commits have been squashed into descriptive, self-contained units of work (e.g. 'WIP' and 'Implements feedback' style messages have been removed)
  • The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)
  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change (find relevant UX in the SMEs section).

/cc @jgiardino

@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Apr 18, 2024

@jeff-phillips-18: GitHub didn't allow me to request PR reviews from the following users: jgiardino.

Note that only opendatahub-io members and repo collaborators can review this PR, and authors cannot review their own PRs.

In response to this:

Closes: RHOAIENG-5491

Description

Show an information section about AI flows in the landing page.

Screen shots

image

image

image

image

How Has This Been Tested?

Tested manually by turning flags on/off.

Test Impact

Added e2e tests.

Request review criteria:

  • The developer has manually tested the changes and verified that the changes work
  • Commits have been squashed into descriptive, self-contained units of work (e.g. 'WIP' and 'Implements feedback' style messages have been removed)
  • The developer has added tests or explained why testing cannot be added (unit or cypress tests for related changes)
  • Included any necessary screenshots or gifs if it was a UI change.
  • Included tags to the UX team if it was a UI/UX change (find relevant UX in the SMEs section).

/cc @jgiardino

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@andrewballantyne

This comment was marked as resolved.

Sign in to view
@jgiardino

This comment was marked as resolved.

Sign in to view
@andrewballantyne
Copy link
Member

Sounds good Jenn... tested and it looks good. @jeff-phillips-18 once you fix up the other comments I think we are good to go.

@jeff-phillips-18 jeff-phillips-18 force-pushed the home-page-ai-flows branch 2 times, most recently from 9effa5a to b83aa22 Compare April 19, 2024 16:36
andrewballantyne
andrewballantyne reviewed Apr 19, 2024
View reviewed changes
frontend/src/pages/home/aiFlows/InfoGallery.tsx Outdated
Comment on lines 11 to 12
const InfoGallery: React.FC<InfoGalleryProps> = ({ infoItems, closeAlt, onClose, closeTestId }) => (
<DividedGallery
Copy link
Member

Choose a reason for hiding this comment

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

What about if (infoItems.length === 0)?

andrewballantyne
andrewballantyne requested changes Apr 22, 2024
View reviewed changes
Copy link
Member

@andrewballantyne andrewballantyne left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Everything works as intended, and I was going to approve this -- then I noticed we have a css small change on selection of the tile and it resizes. It's small, but if we are going to get into custom css, we should do it right.

EDIT: I never actually said what the issue was -- may not be obvious. The height of the element on selection drops 2px and shifts the elements.

Screenshot 2024-04-22 at 9 10 39 AM
Screenshot 2024-04-22 at 9 10 46 AM
Screenshot 2024-04-22 at 9 10 57 AM

@jeff-phillips-18 jeff-phillips-18 force-pushed the home-page-ai-flows branch 3 times, most recently from 33c8674 to 560f8e0 Compare April 22, 2024 15:15
@openshift-ci openshift-ci bot removed the lgtm label Apr 22, 2024
@openshift-ci openshift-ci bot added the lgtm label Apr 22, 2024
@openshift-ci OpenShift CI
Copy link
Contributor

openshift-ci bot commented Apr 22, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: andrewballantyne

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-merge-bot openshift-merge-bot bot merged commit 1e3b295 into opendatahub-io:main Apr 22, 2024
6 checks passed
@jeff-phillips-18 jeff-phillips-18 deleted the home-page-ai-flows branch April 30, 2024 14:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@andrewballantyne andrewballantyne andrewballantyne approved these changes

Assignees

@andrewballantyne andrewballantyne

Labels
approved lgtm
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jeff-phillips-18 @andrewballantyne @jgiardino