Skip to content

Commit 261597c

Browse files
committed
hterm: fix typo in blink preference handling
Commit cce97c4 (hterm: add a helper for setting CSS variables) added a typo with the blink preference. Fix that and rework the code a bit to support tests. Change-Id: I134b375f6548f9e0a838962de173827d3a43950e Reviewed-on: https://chromium-review.googlesource.com/845479 Reviewed-by: David Schneider <[email protected]> Reviewed-by: Brandon Gilmore <[email protected]> Tested-by: Mike Frysinger <[email protected]>
1 parent 31accb6 commit 261597c

File tree

2 files changed

+44
-5
lines changed

2 files changed

+44
-5
lines changed

Diff for: hterm/js/hterm_terminal.js

+22-5
Original file line numberDiff line numberDiff line change
@@ -390,7 +390,7 @@ hterm.Terminal.prototype.setProfile = function(profileId, opt_callback) {
390390
},
391391

392392
'enable-blink': function(v) {
393-
terminal.syncBlinkState();
393+
terminal.setTextBlink(!!v);
394394
},
395395

396396
'enable-clipboard-write': function(v) {
@@ -771,6 +771,20 @@ hterm.Terminal.prototype.setCssVar = function(name, value,
771771
`${opt_prefix}${name}`, value);
772772
};
773773

774+
/**
775+
* Get a CSS variable.
776+
*
777+
* Normally this is used to get variables in the hterm namespace.
778+
*
779+
* @param {string} name The variable to read.
780+
* @param {string?} opt_prefix The variable namespace/prefix to use.
781+
* @return {string} The current setting for this variable.
782+
*/
783+
hterm.Terminal.prototype.getCssVar = function(name, opt_prefix='--hterm-') {
784+
return this.document_.documentElement.style.getPropertyValue(
785+
`${opt_prefix}${name}`);
786+
};
787+
774788
/**
775789
* Set the font size for this terminal.
776790
*
@@ -862,11 +876,14 @@ hterm.Terminal.prototype.syncBoldSafeState = function() {
862876
};
863877

864878
/**
865-
* Enable or disable blink based on the enable-blink pref.
879+
* Control text blinking behavior.
880+
*
881+
* @param {boolean=} state Whether to enable support for blinking text.
866882
*/
867-
hterm.Terminal.prototype.syncBlinkState = function() {
868-
this.setCssVar('node-duration',
869-
this.prefs_.get('enable-blink') ? '0.7s' : '0');
883+
hterm.Terminal.prototype.setTextBlink = function(state) {
884+
if (state === undefined)
885+
state = this.prefs_.get('enable-blink');
886+
this.setCssVar('blink-node-duration', state ? '0.7s' : '0');
870887
};
871888

872889
/**

Diff for: hterm/js/hterm_terminal_tests.js

+22
Original file line numberDiff line numberDiff line change
@@ -477,3 +477,25 @@ hterm.Terminal.Tests.addTest('display-img-invalid', function(result, cx) {
477477

478478
result.requestTime(200);
479479
});
480+
481+
/**
482+
* Verify turning text blink on/off works.
483+
*
484+
* This test isn't great. Since we use CSS animations for everything, we
485+
* assume that part is working, so we just check the stored timing values.
486+
*/
487+
hterm.Terminal.Tests.addTest('text-blink', function(result, cx) {
488+
// Default blink state is enabled.
489+
this.terminal.setTextBlink();
490+
result.assert('0' != this.terminal.getCssVar('blink-node-duration'));
491+
492+
// Explicitly turn it off.
493+
this.terminal.setTextBlink(false);
494+
result.assertEQ('0', this.terminal.getCssVar('blink-node-duration'));
495+
496+
// Explicitly turn it back on.
497+
this.terminal.setTextBlink(true);
498+
result.assert('0' != this.terminal.getCssVar('blink-node-duration'));
499+
500+
result.pass();
501+
});

0 commit comments

Comments
 (0)