Fix clean js from styles 1 #1111
Conversation
idainatovych
force-pushed
the
FIX-clean-js-from-styles-1
branch
from
8e8ae88 to
64bf25b
Compare
idainatovych
force-pushed
the
FIX-clean-js-from-styles-1
branch
from
64bf25b to
1b1b947
Compare
|
|
||
| console.log('recording is visible', isVisible); | ||
|
|
||
| if (isVisible) { |
There was a problem hiding this comment.
The variable above should probably be renamed because if (isVisible) showElement(); doesn't make sense. The console log above also prints the wrong message. The recording is not visible at this point, it will be made visible after we call showElement. Let's call the variable isShow.
There was a problem hiding this comment.
Renamed to shouldShow
| } | ||
|
|
||
| document.querySelector('#recordingSpinner').classList | ||
| .toggle('show-inline', recordingState === Status.RETRYING); |
There was a problem hiding this comment.
As far as I see this line doesn't refactor existing code. Is it a fix for a bug?
There was a problem hiding this comment.
Probably these lines were added after rebase (because refactored version is above)
| } else { | ||
| $("#subject").css({display: "none"}); | ||
| toggleFunction = UIUtil.hideElement.bind(UIUtil); |
There was a problem hiding this comment.
This can be shortened by let toggleFunction = (subject) ? xxxx : yyyy; syntax.
| * @param tag | ||
| * @returns {*} | ||
| */ | ||
| getElementDefaultDisplay(tag) { |
There was a problem hiding this comment.
If this is a private method only let's rename it to _getElement......
There was a problem hiding this comment.
I thought we can expose this method to the interface of UIUtils (it can be useful to define if element is block or inline) but we don't use it in the code outside the module. So, let's keep it private for now.
Do not change element styles directly, use css classes instead. It helps to: