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

Improving the login layout when IE is in intranet mode #15083

Merged
merged 1 commit into from
Jul 12, 2017
Merged

Improving the login layout when IE is in intranet mode #15083

merged 1 commit into from
Jul 12, 2017

Conversation

rhamilto
Copy link
Member

@rhamilto rhamilto commented Jul 6, 2017

@rhamilto rhamilto requested a review from jwforres July 6, 2017 17:00
@jwforres
Copy link
Member

jwforres commented Jul 6, 2017

@rhamilto if media queries are not supported that tells me even more of the console is broken than we realized, if you shrink things down to small desktop is it a mess?

@jwforres
Copy link
Member

jwforres commented Jul 6, 2017

cc @spadgett

@spadgett
Copy link
Member

spadgett commented Jul 6, 2017

@rhamilto Does adding

 <meta http-equiv="X-UA-Compatible" content="IE=edge"> 

to the document head help?

@jwforres
Copy link
Member

jwforres commented Jul 6, 2017 via email

@rhamilto
Copy link
Member Author

rhamilto commented Jul 6, 2017

@spadgett, yep! That's the missing key. The login page does not have the meta element, but the rest of the console does. Will update the PR.

Is there a way for me to test the PR itself? I've been working off local files...

@spadgett
Copy link
Member

spadgett commented Jul 6, 2017

You can try

$ OS_BUILD_ENV_PRESERVE="_output/local" hack/env OS_ONLY_BUILD_PLATFORMS="linux/amd64" WHAT=cmd/openshift make
$ hack/build-local-images.py origin
$ oc cluster up --version=latest

@jwforres
Copy link
Member

jwforres commented Jul 6, 2017

@rhamilto so we actually have several other templates for the oauth server, and I guarantee they probably are ALL missing this line

@jwforres
Copy link
Member

jwforres commented Jul 6, 2017

@liggitt FYI since this will be touching all of the oauth server's HTML pages (will wait for 3.7)

@@ -83,6 +83,7 @@ const defaultLoginTemplateString = `<!DOCTYPE html>
<head>
<title>Login - OpenShift Origin</title>
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<meta http-equiv="X-UA-Compatible" content="IE=edge">
Copy link
Member

Choose a reason for hiding this comment

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

This should be the first meta tag. Best to put it immediately after head.

@rhamilto rhamilto changed the title [WIP] Improving the login layout when IE is in compatibility mode (IE7) [WIP] Improving the login layout when IE is in intranet mode Jul 6, 2017
@rhamilto rhamilto changed the title [WIP] Improving the login layout when IE is in intranet mode Improving the login layout when IE is in intranet mode Jul 6, 2017
@jwforres
Copy link
Member

jwforres commented Jul 6, 2017

@rhamilto so @liggitt also has an oauth access granting page which isn't based off this same theme, but could still have issues, dont remember what the name of that file is offhand, maybe @deads2k knows?

@rhamilto
Copy link
Member Author

rhamilto commented Jul 6, 2017

@rhamilto so @liggitt also has an oauth access granting page which isn't based off this same theme, but could still have issues, dont remember what the name of that file is offhand

I think it's pkg/auth/server/grant/templates.go

I've searched the repo for the following and updated accordingly:

contain "data:image"

  • pkg/auth/server/errorpage/templates.go [UPDATED]
  • pkg/auth/server/login/templates.go [UPDATED]
  • pkg/auth/server/selectprovider/templates.go [UPDATED]

contain "@media"

  • cmd/cluster-capacity/go/src/github.com/kubernetes-incubator/cluster-capacity/* (documentation pages)
  • examples/atomic-registry/allinone/registry-login-template.html [UPDATED]
  • examples/atomic-registry/systemd/templates/registry-login-template.html [UPDATED]
  • pkg/auth/server/errorpage/templates.go [UPDATED]
  • pkg/auth/server/grant/templates.go [UPDATED]
  • pkg/auth/server/login/templates.go [UPDATED]
  • pkg/auth/server/selectprovider/templates.go [UPDATED]
  • pkg/auth/server/tokenrequest/endpoints.go (not a full html file and only contains one rule--.nowrap)

I think we should be good.

<title>
Authorize
Authorize
Copy link
Member

Choose a reason for hiding this comment

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

probably want to be careful about these whitespace changes, it might be affecting the final output

@jwforres
Copy link
Member

jwforres commented Jul 6, 2017

[test]

@openshift-bot
Copy link
Contributor

Evaluated for origin test up to 8d3eb02

@jwforres
Copy link
Member

jwforres commented Jul 6, 2017

I think we might get a clean merge of this change with the existing OCP template overrides, so we may not need a separate PR for OCP

@openshift-bot
Copy link
Contributor

continuous-integration/openshift-jenkins/test SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_request_origin/2998/) (Base Commit: 289cf8f) (PR Branch Commit: 8d3eb02)

@liggitt
Copy link
Contributor

liggitt commented Jul 10, 2017

LGTM

@smarterclayton
Copy link
Contributor

Low risk, big win [severity:blocker] [merge]

@openshift-bot
Copy link
Contributor

openshift-bot commented Jul 12, 2017

continuous-integration/openshift-jenkins/merge Waiting: You are in the build queue at position: 3

@openshift-bot
Copy link
Contributor

Evaluated for origin merge up to 8d3eb02

@smarterclayton smarterclayton merged commit c2b06c7 into openshift:master Jul 12, 2017
@rhamilto rhamilto deleted the bz-1298750 branch July 12, 2017 13:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants