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

Restore missing WebUI breadcrumbs after refactor #8332

Merged
merged 2 commits into from
May 22, 2015
Merged

Restore missing WebUI breadcrumbs after refactor #8332

merged 2 commits into from
May 22, 2015

Conversation

yllierop
Copy link
Contributor

  • Update paths to svg images
  • Restore sections concept
  • Tested with local cluster via hack/local-up-cluster.sh

@yllierop
Copy link
Contributor Author

@lavalamp can you please review this change:
pr_8332

@yllierop
Copy link
Contributor Author

Once these changes #8302 are merged the links will function correctly again. The move from running in a pod to running directly from the API Server seems to have definitely caused some issues.

@yllierop yllierop mentioned this pull request May 15, 2015
12 tasks
@bgrant0607
Copy link
Member

cc @jackgr

 - Update paths to svg images
 - Restore sections concept
 - Tested with local cluster
- Update back buttons
- Ignore duplicated README.md
- Rename /minions to /nodes
- Deactivate more buttons
- Updates to list selection and node detail page
@lavalamp
Copy link
Member

Can you add a test to ensure that this doesn't get broken in the future?

Also, the bindata go file seems to have gotten its boilerplate header messed up, which is why travis & shippable are mad at you. Please fix it it (for extra credit, fix the thing that generated it?)

@jackgr
Copy link
Contributor

jackgr commented May 18, 2015

Breadcrumbs and back button still don't work correctly:

  1. Broken breadcrumbs:
    • Start the app.
    • Click Views/Services. View changes to Services. Breadcrumbs show Dashboard > Services. Click Dashboard. View changes to Dashboard. OK so far, but...
    • Breadcrumbs still show Dashboard > Services.
  2. Broken back button:
    • Start the App
    • Click Views/Nodes. View changes to Nodes. Click a node. See the Back button. OK so far, but...
    • Click the Back button. View changes to Services, instead of going back to Groups.

@yllierop
Copy link
Contributor Author

@lavalamp it looks like currently that hack/build-ui.sh is just using cat hooks/boilerplate.go.txt > $TMP_DATAFILE to insert the boilerplate. I've manually fixed the date in the header for now. But a cleaner solution is in the works. @jackgr I'm reviewing your issue and I'm able to replicate it locally. I'll work on a fix shortly. By chance have you confirmed the same behavior in the older versions as well?

@jackgr
Copy link
Contributor

jackgr commented May 18, 2015

@preillyme have not checked older versions.

@yllierop
Copy link
Contributor Author

@jackgr Okay I'll check the older version shortly. I'm curious if it was always broken or just broke now. I suspect it's something with: https://docs.angularjs.org/api/ng/service/$location

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

@bcbroussard
Copy link
Contributor

@jackgr PTAL, I've added another commit to address the issues in your comment, along with a few other minor updates.

  • Update pod list back button
  • Ignore duplicated README.md
  • Rename /minions to /nodes
  • Deactivate more buttons
  • Updates to list selection and node detail page

@yllierop
Copy link
Contributor Author

@bcbroussard
Copy link
Contributor

@googlebot I’m okay with these commits https://github.com/preillyme/kubernetes/commit/4ce6783a8ac3ad1a9a4e15a7619f7c8588cf8e18 being contributed to Google

@yllierop
Copy link
Contributor Author

pr_8332

This now looks good to me. @lavalamp can you take a look please.

@jackgr
Copy link
Contributor

jackgr commented May 20, 2015

👍

@yllierop
Copy link
Contributor Author

@lavalamp Can you please review this PR?

@bgrant0607
Copy link
Member

@jackgr reviewed, so I'm going to put lgtm on this.

@bgrant0607 bgrant0607 added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 21, 2015
bcbroussard referenced this pull request in bcbroussard/kubernetes May 21, 2015
@dchen1107
Copy link
Member

Please rebase so that I can merge your pr, sorry for the inconvienence.

@lavalamp
Copy link
Member

Needs rebase.

@lavalamp lavalamp removed the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2015
@lavalamp
Copy link
Member

Also this seems light on tests? Should I not worry about that?

@googlebot
Copy link

CLAs look good, thanks!

@yllierop
Copy link
Contributor Author

@lavalamp I've rebased. Yes this is also pretty light on tests but we've got some other changes around testing coming soon in another PR. So I wouldn't worry about it in this PR.

@lavalamp lavalamp added the lgtm "Looks good to me", indicates that a PR is ready to be merged. label May 22, 2015
dchen1107 added a commit that referenced this pull request May 22, 2015
Restore missing WebUI breadcrumbs after refactor
@dchen1107 dchen1107 merged commit 1131ac9 into kubernetes:master May 22, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
lgtm "Looks good to me", indicates that a PR is ready to be merged.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants