SCAYT's context menu option "more suggestions" doesn't fit

[Piotrek Koszuliński]

1. Enable SCAYT.
2. Right click on misspelled word.
3. "More suggestions" is hidden under the arrow on the right.

[Olek Nowodziński]

Created branch t/9248 with the solution.

[Frederico Caldeira Knabben]

Unfortunately the proposed solution (read it as ugly hack) is not acceptable. We need instead to be focused on fixing it by proposing DOM+CSS changes.

[Olek Nowodziński]

Alright, but DOM changes might result in totally new context menu since I've already tried to gently fix it with CSS and I failed. The problem is that the current context menu implementation is very limited and it's hard to change something but not to change the entire template which is, of course, something undesirable.

[Olek Nowodziński]

Created branch t/9248c with another, DOM-oriented approach. This time it's not so dirty. I'd rather say it's tricky. Please consider all the browsers and HC mode when reviewing to avoid regressions.

[Garry Yao]

After the patch, arrow is not showing in long label text in RTL on the first open, check the attached sample to reproduce.

[Garry Yao]

I've pushed t/9248b, another approach that use positioning, it's less tricky and simpler, the only exception is that it will not work well for RTL in IE7 because of a current regression.

[Olek Nowodziński]

Replying to garry.yao:

I've pushed t/9248b, another approach that use positioning, it's less tricky and simpler, the only exception is that it will not work well for RTL in IE7 because of a current regression.

Thank you for the contribution, Garry! However I finally reached desired look among all the browsers except Opera, which is broken much more than this. I rebased t/9248c on t/9355 (#9355) with a small fix and it's fine now.

[Garry Yao]

Ok, the result looks good for t/9248c, which is a good rescue for IE7, while it still breaks Opera & IE quirks, which looks fine for master at least.

The think is that we must keep in mind to have a clean main CSS, for good browsers, the quirks that are affecting bad browsers should be dedicated to their own file, read as the reason why we create the browser skin part feature, and to accelerate skin developing (not forcing them to consider browser compatibility at the first time), so in this case, float is only required for IE7 (to stretch properly the menu in RTL).

I've ported the float solution proposed by t/9248c, on top of t/9248b, for IE7, I'd ask to switch the review on t/9248c at this point.

I've R+ and landed #9355 to accelerate the review.

[Olek Nowodziński]

Thanks for #9355! t/9248 looks good. This is something we can use if other methods fail.

Anyway, I discovered a completely different approach which is based on display: table-* and I pushed this to t/9248d. No floating, it's intuitive, and the only hacks are for IE7.

• Works everywhere in LTR and RTL (IE7+, FF3.6, FF latest, Chrome).
• The only exception is Opera. It fails in RTL. I think it's either Opera bug or something's wrong with ​these lines. It'd be great if you, Garry, tried to figure this out as it's pretty weird for me because this display:table-* works fine in Opera while LTR is set.
• t/9248d is a pre-review thing as HC hasn't been implemented yet to save time.
• Check this out ;)

I hope we'll handle this Opera RTL thing, finish HC, and this solution will be final.

PS: We could eventually use <table> markup as a solution as well, which might be more bulletproof than display:table-*.

[Garry Yao]

Brilliant approach， table is very suitable CSS presentation of this layout requirement as well, but it imposed an extra "cke_menubutton_wrap" just for putting a "display:table" on it, which I can tell is just not required, "row" and "cell" is enough to make it right.

[Olek Nowodziński]

Updated t/9248d:

• Removed cke_menubutton_wrap. It was unnecessary, thanks for pointing this out!
• Fixed HC mode.

Please, test all the possible configurations (browser+RTL+HC) as this is a crucial part of UI.

The only bug I see is a weird RTL in Opera.

PS 1. Do we care about quirks? PS 2. If R+, I'll squash it a little bit.

[Garry Yao]

Good to go with t/9248d.

[Olek Nowodziński]

Fixed by the ​commit.