Skip to content

Update site nav with openshift.com redesign #9032

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

Merged
merged 1 commit into from
May 4, 2018
Merged

Conversation

wgordon17
Copy link
Contributor

No description provided.

@openshift-ci-robot openshift-ci-robot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label May 3, 2018
@wgordon17
Copy link
Contributor Author

@wgordon17
Copy link
Contributor Author

@nhr or @vikram-redhat Could I please get a review and merge for this? Thanks!

@nhr
Copy link
Member

nhr commented May 3, 2018

@wgordon17 - I don't think Vikram or I are the best people to review these changes; they are all layout changes to match the new OpenShift.com design. I would suggest that you ask if @luciddreamz can do the review or point you in the direction of someone else who can do that style comparison.

@luciddreamz
Copy link

@nhr The changes look correct, in line with the new www.openshift.com website navigation, and only impact commercial docs. I assume the updates to the contributing process are also accurate, but I can't comment on that part beyond assuming the update makes sense.

@nhr
Copy link
Member

nhr commented May 3, 2018

Ack, thanks for the review @luciddreamz !
@wgordon17 I think you are good to go. I'll let @vikram-redhat have a shot and he can merge it if he's good with the content.

@vikram-redhat
Copy link
Contributor

@wgordon17 @luciddreamz @nhr - all looks good to me, but just wondering if we can increase the space between the black header at the top and the breadcrumbs that follow immediately?

For example: https://commercial-docs-wgordon.6923.rh-us-east-1.openshiftapps.com/online/welcome/index.html

@wgordon17
Copy link
Contributor Author

@vikram-redhat I had some margin included (https://github.com/openshift/openshift-docs/pull/9032/files#diff-e93b21cedf1f2b7dfd23be811beadf80), but maybe it has to do with the way I packaged it, that it didn't get applied.

Since I'm only updating master, I would assume that once this is cherry picked to all of the branches, it would work as expected, right?

@vikram-redhat
Copy link
Contributor

@wgordon17 - ah yes, it should get applied as the stylesheets are picked from the master branch if you build from the master branch. @nhr can correct me if I am wrong.

Other than that it looks good to me.

I am technically on PTO this week and Monday is a public holiday in Brisbane. If this needs to be merged before the summit, and if I am not around or can't get access to my computer, just ping anyone in the team openshift docs to merge.

cc: @aheslin FYI.

@nhr
Copy link
Member

nhr commented May 4, 2018

@wgordon17 yes, css changes need to be cherry-picked to the branches.

@nhr
Copy link
Member

nhr commented May 4, 2018

I've hand-merged the docs.css change to all non-master branches; merging this now.

@nhr nhr merged commit 0ce493a into openshift:master May 4, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants