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.

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

Add fullscreen terminal support #1167

Merged
merged 1 commit into from
Feb 22, 2017
Merged

Add fullscreen terminal support #1167

merged 1 commit into from
Feb 22, 2017

Conversation

spadgett
Copy link
Member

@spadgett spadgett commented Jan 21, 2017 •
edited
Loading

@jwforres FYI I have fullscreen support working for terminals. Need to test more across browsers.

@csrwng CC

TODO:

  • Add expand and collapse icons for entering and exiting fullscreen (fa-expand / fa-compress)
  • Investigate sending ESC to the terminal since it's trapped by the browser to exit fullscreen

@spadgett spadgett force-pushed the fullscreen branch 6 times, most recently from 67dc20d to fb64804 Compare January 23, 2017 13:49
@openshift-bot openshift-bot added the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 24, 2017
jwforres
jwforres reviewed Jan 24, 2017
View reviewed changes
app/scripts/controllers/pod.js Outdated
@@ -316,6 +317,17 @@ angular.module('openshiftConsole')
}
};

var focusTerminal = function() {
$('.terminal').focus();
Copy link
Member

Choose a reason for hiding this comment

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

do we actually have multiple terminals when there are multiple containers (with only one visible)? or do we just maintain the connections?

or does jquery only find the visible ones by default, cant remember

Copy link
Member Author

Choose a reason for hiding this comment

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

Good catch. It looks like we keep the terminal once it's been used, so it's possible to have multiple. I'll change it to only focus the visible terminal.

@spadgett spadgett changed the title [WIP] Add fullscreen terminal support Add fullscreen terminal support Jan 24, 2017
@spadgett
Copy link
Member Author

Updated

@openshift-bot openshift-bot added needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. and removed needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. labels Jan 24, 2017
@openshift-bot openshift-bot removed the needs-rebase Indicates a PR cannot be merged because it has merge conflicts with HEAD. label Jan 24, 2017
@spadgett spadgett force-pushed the fullscreen branch 3 times, most recently from fc32691 to 5cbafe8 Compare January 27, 2017 20:54
@spadgett
Copy link
Member Author

Rebased

@jwforres
Copy link
Member

[merge]

@openshift-bot
Copy link

Evaluated for origin web console merge up to a6d44e8

@openshift-bot
Copy link

[Test]ing while waiting on the merge queue

@openshift-bot
Copy link

Evaluated for origin web console test up to a6d44e8

@openshift-bot
Copy link

Origin Web Console Test Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1052/) (Base Commit: e812e57)

@openshift-bot
Copy link

openshift-bot commented Feb 22, 2017 •
edited
Loading

Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/1067/) (Base Commit: 5b16b10)

@openshift-bot openshift-bot merged commit 36c701f into openshift:master Feb 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jwforres jwforres jwforres approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@spadgett @jwforres @openshift-bot