-
Notifications
You must be signed in to change notification settings - Fork 167
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
Revamp empty state in Model Serving Global #1796
Conversation
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
@kaedward this is the end result: I went with a small I only have one question here, the |
await page.goto( | ||
'./iframe.html?args=&id=tests-integration-pages-modelserving-modelservingglobal--delete-model&viewMode=story', | ||
navigateToStory('modelserving-modelservingglobal', 'empty-state-no-serving-runtime'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've just created this to open a conversation, I think @andrewballantyne mentioned this a few weeks ago, but I couldn't find any logged issue, should we include this util method to keep the navigation more consistent?
@lucferbux here's some updated text for this empty state: "No deployed models yet As for the layout questions, I'm not sure - maybe @vconzola would have an opinion :-) |
dc1f4ca
to
2b3949d
Compare
@lucferbux please recreate your PR, I have reverted this in #1834 |
Description
Closes #1091
How Has This Been Tested?
Model Serving
global pageTest Impact
Added coverage for empty state
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
main