Skip to content

Conversation

@Tyriar
Copy link
Member

@Tyriar Tyriar commented Jan 14, 2017

Fixes #478

@Tyriar Tyriar added this to the 2.3.0 milestone Jan 14, 2017
@Tyriar Tyriar self-assigned this Jan 14, 2017
@Tyriar Tyriar requested a review from parisk January 14, 2017 05:31
@parisk
Copy link
Contributor

parisk commented Jan 14, 2017

What if we changed cursorBlink to work purely with CSS (and CSS animations)?

This way we will avoid redrawing.

@Tyriar
Copy link
Member Author

Tyriar commented Jan 14, 2017

We do control it completely with CSS. Now that you say that we could grab the cursor element if it's available and apply the class to it. Not sure it's worth duplicating the cursor logic since we're only refreshing a single line which is a pretty fast operation anyway.

@parisk
Copy link
Contributor

parisk commented Jan 14, 2017

We can set the CSS class on term.element and add the appropriate CSS attributes based on the following-rule (or sth like that): .xterm.xterm-cursor-blink .terminal-cursor.

@Tyriar
Copy link
Member Author

Tyriar commented Jan 14, 2017

I saw there is an escape sequence which handles changing the cursor style. I wonder if it would be useful to support that? Maybe existing config files could set it then?

@parisk
Copy link
Contributor

parisk commented Jan 14, 2017

I think we should support it via CSS as well.

@parisk
Copy link
Contributor

parisk commented Jan 15, 2017

Works great. Also tests have passed, but have not been reported: https://travis-ci.org/sourcelair/xterm.js/builds/191852761.

Only thing needed is a rebase with master.

@Tyriar Tyriar force-pushed the 478_cursorBlink_refresh branch from f9769d2 to 12160a5 Compare January 15, 2017 10:32
@Tyriar Tyriar merged commit 4d5046b into xtermjs:master Jan 15, 2017
@Tyriar Tyriar deleted the 478_cursorBlink_refresh branch January 15, 2017 10:35
@Tyriar Tyriar modified the milestone: 2.3.0 Feb 2, 2017
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.

2 participants