Skip to content

Refactor tabs implementation #3268

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
Merged
merged 35 commits into from
May 4, 2017
Merged

Refactor tabs implementation #3268

merged 35 commits into from
May 4, 2017

Conversation

@chenopis
Copy link
Contributor

@chenopis chenopis commented Apr 7, 2017 •
edited by k8s-reviewable
Loading

Improve tabs implementation w/ these requirements:

  • CSV field you specify to name the tab, assumes no spaces in the name
    • Having a name with spaces results in a broken page
    • Requirement: need to be able to have tab names with spaces in them (e.g. "Weave Net")
  • Want to have no default selected, default should say "select one of the tabs"
    • In particular, when no default is selected, it shouldn't be a code block
  • Current implementation requires file that needs to be linked to
    • One line doesn't require a file, nor a link to that file

This change is Reviewable

@chenopis chenopis added the do-not-merge DEPRECATED. Indicates that a PR should not merge. Label can only be manually applied/removed. label Apr 7, 2017
@k8s-ci-robot k8s-ci-robot added the cncf-cla: yes Indicates the PR's author has signed the CNCF CLA. label Apr 7, 2017
@ahmetb
Copy link
Member

ahmetb commented May 2, 2017

@chenopis I'm trying to understand why you need to add bunch of webfonts assets and a bootstrap.js that we don’t use to fix this problem.

@chenopis
Copy link
Contributor Author

chenopis commented May 2, 2017

@ahmetb I haven't actually had a chance to work on this yet. I was in the middle of adding stuff to the bootstrap framework to try out their implementation of tabs. Just ignore this PR for now until the WIP: prefix has been dropped.

@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch from 6d8a734 to 52bcfb8 Compare May 2, 2017 22:30
@chenopis
Merge branch 'master' of https://github.com/kubernetes/kubernetes.git…
a9cafbc 
…hub.io into chenopis-weaveworks-tabs

* 'master' of https://github.com/kubernetes/kubernetes.github.io: (258 commits)
  Fixes broken links
  Removed deleted links regarding minikube CLI
  Add missing volumetype to pv doc (#3600)
  Fix broken link in HA doc with minikube (#3599)
  rolling-update: remove link to the doc itself
  Link to 'why containers' in kubernetes-basics
  Updating Ubuntu based instructions and upgrades
  Address Issue #3577
  Added more explanation for nodeAffinity.
  Update and fix travis tests (short-term)
  Address #3433 - update Tasks landing page
  Remove redundant, out of sync table of contents
  Update Init Container link
  Update service.md
  Link to appropriate docs for Automatic Mounting of Manually Created Secrets (#3571)
  update kubectl apply doc for required resource name
  jlx426
  Updating Canal URL (#3583)
  Grammar patch to #3572
  Remove swagger-ui from search index
  ...
@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch from 52bcfb8 to a9cafbc Compare May 2, 2017 22:37
@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch 2 times, most recently from d6820a7 to 1a7d7ac Compare May 2, 2017 23:44
@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch from 1a7d7ac to 26056c7 Compare May 3, 2017 15:34
@chenopis
Merge branch 'master' of https://github.com/kubernetes/kubernetes.git…
6576b97 
…hub.io into chenopis-weaveworks-tabs

* 'master' of https://github.com/kubernetes/kubernetes.github.io:
  Fix Travis CI
  fix redirect link (#3568)
  Remove duplicate redirect. (#3613)
  Fix typo in redirect. (#3611)
@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch 4 times, most recently from bf580ec to 867bc07 Compare May 3, 2017 17:58
@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch from 867bc07 to 92ff0f4 Compare May 3, 2017 18:11
@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch 3 times, most recently from 6a0ab5e to 2ed57f9 Compare May 3, 2017 22:13
@chenopis chenopis mentioned this pull request May 4, 2017
Closed
ahmetb
ahmetb approved these changes May 4, 2017
View reviewed changes
Copy link
Member

@ahmetb ahmetb 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

@chenopis
Copy link
Contributor Author

chenopis commented May 4, 2017

Ok, cool. I'm just going to add some documentation to the example and call it a day.

@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch 8 times, most recently from f97b5fb to f75b96f Compare May 4, 2017 19:25
@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch from f75b96f to 029d635 Compare May 4, 2017 19:31
chenopis added 3 commits May 4, 2017 13:32
@chenopis
move demo to top, pretty it up
3264051
@chenopis
add word
2fdbe0e
@chenopis
Merge branch 'master' of https://github.com/kubernetes/kubernetes.git…
c349c01 
…hub.io into chenopis-weaveworks-tabs

* 'master' of https://github.com/kubernetes/kubernetes.github.io:
  Move to Tasks: Replicated Stateful. (#3639)
  Move to Tasks: Single-instance Stateful. (#3638)
  Combine topic on commands and args. (#3637)
  Move topic to Inject Data section. (#3635)
  Move topic to Injecting Data section. (#3634)
@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch from b6d77ee to 772bf1f Compare May 4, 2017 20:42
@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch from 772bf1f to ecd6348 Compare May 4, 2017 20:44
@chenopis chenopis force-pushed the chenopis-weaveworks-tabs branch from ecd6348 to 61ce5a5 Compare May 4, 2017 20:46
@chenopis chenopis merged commit 105942f into master May 4, 2017
@chenopis chenopis deleted the chenopis-weaveworks-tabs branch May 4, 2017 20:53
@chenopis
Copy link
Contributor Author

chenopis commented May 4, 2017

@errordeveloper If you see any issues, I can address them in a follow-up PR. Cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@errordeveloper errordeveloper Awaiting requested review from errordeveloper

1 more reviewer

@ahmetb ahmetb ahmetb approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

@errordeveloper errordeveloper

@lukemarsden lukemarsden

@chenopis chenopis

Labels

cncf-cla: yes Indicates the PR's author has signed the CNCF CLA.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@chenopis @ahmetb @errordeveloper @lukemarsden @k8s-ci-robot