Refactor hiding-methods, remove jQuery show/hide, remove .hide class, remove inline style=display:none

[wxiaoguang]

Close #22847

This PR:

• introduce Gitea's own showElem and related functions
• remove jQuery show/hide
• remove .hide class
• remove inline style=display:none

From now on:

do not use:

• "[hidden]" attribute: it's too weak, can not be applied to an element with "display: flex"
• ".hidden" class: it has been polluted by Fomantic UI in many cases
• inline style="display: none": it's difficult to tweak
• jQuery's show/hide/toggle: it can not show/hide elements with "display: xxx !important"

only use:

• this ".gt-hidden" class
• showElem/hideElem/toggleElem functions in "utils/dom.js"

cc: @silverwind , this is the all-in-one PR

[silverwind]

This does look promising, hope it doesn't break stuff :D

[wxiaoguang]

Why? silverwind ***@***.***>

…

***@***.**** commented on this pull request. ------------------------------ In web_src/js/utils/dom.js <#22950 (comment)>: > @@ -0,0 +1,65 @@ +function getComputedStyleProperty(el, prop) { + const cs = el ? window.getComputedStyle(el) : null; + return cs ? cs[prop] : null; +} + +function isShown(el) { + return getComputedStyleProperty(el, 'display') !== 'none'; +} classList.contains? — Reply to this email directly, view it on GitHub <#22950 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQEFDKB3VONTDB264N4UVDWX4YOHANCNFSM6AAAAAAU7BAESI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[wxiaoguang]

Move to where? silverwind ***@***.***>

…

***@***.**** commented on this pull request. ------------------------------ In web_src/js/utils/dom.js <#22950 (comment)>: > @@ -0,0 +1,65 @@ +function getComputedStyleProperty(el, prop) { + const cs = el ? window.getComputedStyle(el) : null; + return cs ? cs[prop] : null; +} + +function isShown(el) { + return getComputedStyleProperty(el, 'display') !== 'none'; +} + +function assertShown(el, expectShown) { + if (window.config.runModeIsProd) return; Better to move this outside for a tiny perf gain as JS does not have to call the function every time. — Reply to this email directly, view it on GitHub <#22950 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAQEFDPS5BQ5YH55SQYBNPTWX4YSJANCNFSM6AAAAAAU7BAESI> . You are receiving this because you authored the thread.Message ID: ***@***.***>

[silverwind]

Why?

When all show/hide mechanism rely on class name, we don't need the expensivegetComputedStyle and can just check class existance.

Move to where?

Move the window.config.runModeIsProd check to the call site.

[wxiaoguang]

Why?

When all show/hide mechanism rely on class name, we don't need the expensivegetComputedStyle and can just check class existance.

No, that's not what it works.

getComputedStyle is only a fallback (debug assertion) check to detect whether there are something like style=display:none or .popup {display:none}, to catch bugs as early as possible.

You can not replace it with contains.

Move to where?

Move the window.config.runModeIsProd check to the call site.

It will make code quite complex and dirty (you can have a try).

And see the comment: "this assertion can be removed after next release cycle or if it has been proved that there is no bug."

[silverwind]

OK I guess if that function is not here forever, it's better to keep the check inside it.

[delvh]

Found no errors during my testing.