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

Polish changes: #2733

Merged
merged 4 commits into from
Mar 20, 2019
Merged

Polish changes: #2733

merged 4 commits into from
Mar 20, 2019

Conversation

avdaredevil
Copy link
Contributor

@avdaredevil avdaredevil commented Mar 19, 2019

fixes #2359

Features

  • List box text was not visible in blue toolbar mode
  • Colors adjusted for clearer viewing of namespace selector
  • Made the message much easier to read and made it an empty state message

/assign @avdaredevil @prodonjs
/review @prodonjs
/priority p0


This change is Reviewable

- List box text was not visible in blue toolbar mode
- Colors adjusted for clearer viewing of namespace selector
- Made the message uch easier to read and made it an empty state message
Copy link
Contributor

@prodonjs prodonjs left a comment

Choose a reason for hiding this comment

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

Looks good just one comment. Also, while you're looking at it, I noticed that it might be useful to add some minimum height to the activity-rows to keep them consistent.

https://screenshot.googleplex.com/SQYG8pNe1zT.png

@avdaredevil
Copy link
Contributor Author

avdaredevil commented Mar 19, 2019

Hey @prodonjs can you please test gcr.io/kubeflow-images-public/centraldashboard:v20190318-v0.4.0-rc.1-237-gb0f6b6a0 it seems a reload on a custom route fails.

Edit:

Missed this, can we tackle this in a later change, I won't be able to work on this any longer, and would like to make it for the v0.5.0. So the critical stuff is in at least?

@prodonjs
Copy link
Contributor

/test kubeflow-presubmit

@prodonjs
Copy link
Contributor

I didn't see any issues with the deployed image when trying to navigate directly to a deep-link (ie. https://kubeflow.endpoints.prodonjs-kubeflow-dev.cloud.goog/_/notebooks) or to a non-existent route (ie. https://kubeflow.endpoints.prodonjs-kubeflow-dev.cloud.goog/a/bad/route).

The presubmit failure is unrelated to the change here as our image is building correctly.

/lgtm
/approve

@prodonjs
Copy link
Contributor

/assign @gaocegege

Need owners approval for jsonnet file change.

@kunmingg
Copy link
Contributor

/lgtm

@kunmingg
Copy link
Contributor

/approve

@avdaredevil avdaredevil mentioned this pull request Mar 19, 2019
22 tasks
@kunmingg
Copy link
Contributor

@avdaredevil
#2729 merged, can you rebase?

@k8s-ci-robot k8s-ci-robot removed the lgtm label Mar 20, 2019
@avdaredevil
Copy link
Contributor Author

avdaredevil commented Mar 20, 2019 via email

@kunmingg
Copy link
Contributor

/lgtm
/approve

@k8s-ci-robot
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: kunmingg, prodonjs

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

@k8s-ci-robot k8s-ci-robot merged commit cc2e17c into kubeflow:master Mar 20, 2019
@avdaredevil avdaredevil deleted the final-polish branch March 21, 2019 00:13
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 11, 2021
* Polish changes:
- List box text was not visible in blue toolbar mode
- Colors adjusted for clearer viewing of namespace selector
- Made the message uch easier to read and made it an empty state message

* Addressed prodonjs' comments

* Updated image
saffaalvi pushed a commit to StatCan/kubeflow that referenced this pull request Feb 12, 2021
* Polish changes:
- List box text was not visible in blue toolbar mode
- Colors adjusted for clearer viewing of namespace selector
- Made the message uch easier to read and made it an empty state message

* Addressed prodonjs' comments

* Updated image
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.

[Dashboard v1][Tracker][Redesign] Landing Page v1
6 participants