Drop down is not "on" when clicking it while editor is blurred

[Piotrek Koszuliński]

1. Open replacebycode sample.
2. Click styles drop down without focus editor.
3. Drop down opens but button is "off".

Reproduced on Chrome and IE11, should be producible on all browsers.

First bad commit ​git:094859c.

[Piotr Jasiun]

Fixed with tests and small clean up.

Changes in ​t/11793 and corresponding test branch.

[Piotrek Koszuliński]

Couldn't we simplify it? I pushed ​branch:t/11793b. What do you think about it?

[Piotr Jasiun]

Well, in your solution there is if condition, then ternary operator which set status and then another if which may change this status. In my there is one switch with 4 cases. In my opinion one big switch is simpler, but it's a matter of taste.

[Piotr Jasiun]

And I'm not sure if you solution works in the best way, because if editor is readOnly the status should be DISABLED even if it was ON. I know that is very unlikely to change editor to the readOnly mode when the dropdown is open, but I think that it is the good way to check if the editor is in readOnly before we check if we should leave combo in ON state.

[Piotrek Koszuliński]

The ​branch:t/11793b adds an early return, which I think is very clear in this case. There's the validation if the function should be executed and then its the logic (not touched by this branch). Additionally, the early return saves some time, because e.g. in t/11793 setState and setValue are called even though none should be changed (if combo is ON).

Regarding the read only case - test it ;). If you have opened combo and something sets the read only to true, the combo is still opened, so the state should be on. The state may only be changed when combo is closed (although the best solution is to close the dropdown but this is out of this ticket's scope). So the t/11793b works ok, when t/11793 works incorrectly.

BTW. A test would be welcomed.

[Piotr Jasiun]

I tested you your solution and it works fine. I moved tests from t/11793 and simplified them using bot.combo. Changes in ​t/11793b and corresponding test branch.

Regarding the read only case - I think that it is a different problem, not related with this ticket, and the scenario is so uncommon (someone have to change readonly mode with open combo) that it has pretty low priority.

[Piotrek Koszuliński]

Details, details, details.

1. The test file is titled 'Plugin: stylescombo'.
2. Uppercase letter is used in file name.
3. The test file subtitle 'update state' is unclear. What is it supposed to contain? Note also that the readonly.html also test... updating state. I think that one file containing both tests, called state.html will make more sense.
4. Test case title 'test clicking while blurred' is unclear too. It sounds like the button is blurred, when it means that editor has to be blurred.

[Piotr Jasiun]

I merged both tests into state.html and updated tests descriptions. Changes in 1/11793b test branch.

[Piotrek Koszuliński]

Remove the //]]> comment from test file before closing branch.

[Piotr Jasiun]

Done and closed.

• ​git:cce1107
• tests:33d9681