Kuma: Block indicator icons are broken on the React‑based frontend

Created on 24 Oct 2019  ·  6Comments  ·  Source: mdn/kuma

Summary

Block indicator icons are broken, and sometimes duplicated, on the React‑based MDN frontend.

Steps To Reproduce (STR)

  1. Go to the React‑based Indicator sandbox page.
  2. Compare with the Django‑based Indicator sandbox page.

Expected behavior

The block indicator icons have the same position and aren’t duplicated.

Actual behavior

Because the CSS files are compiled in a different order, the block indicator icons on the React‑based frontend have wrong margins, and sometimes are duplicated.


Screenshot

Screenshot_2019-10-24 Indicator sandbox

Additional context

The duplicate icons can be fixed by adding !important to the display property in: https://github.com/mdn/kuma/blob/43db78ccea993ad8e5f567154d645d34e006f5d4/kuma/static/styles/includes/_mixins_indicators.scss#L93-L99

This is being done in #6036.

A much better way to do that would be to get rid of the unnecessary {{*_Inline}} macro calls in the {{*_Header}} macros, which I’m also doing in https://github.com/mdn/kumascript/pull/963, https://github.com/mdn/kumascript/pull/983, https://github.com/mdn/kumascript/pull/984 and https://github.com/mdn/kumascript/pull/987.

p2

All 6 comments

The duplicate icons can be fixed by adding !important to the display property in:

I would definitely not want us to go this route.

A much better way to do that would be to get rid of the unnecessary {{*_Inline}} macro calls in the {{*_Header}} macros, which I’m also doing in mdn/kumascript#963, mdn/kumascript#983, mdn/kumascript#984 and mdn/kumascript#987.

Should this by itself solve the problem and remove the need for !important I would recommend we go with this solution. Thank you @ExE-Boss

That would remove the need for the whole display: none rule, after pages get re‑rendered.

How come the Wiki doesn't have the problem? Does the wiki already use the rather hacky display: none !important trick?

I see 4 monster PRs that might take years to review. Are you sure there's not a way to greatly simplify those into tiny PRs that are more likely going to get review attention @ExE-Boss ?

No, the wiki CSS has the display: none and .⁠<block‑indicator‑with‑icon>::before rules located after the rules that set display: inline‑block and margin properties respectively with the same specificity in the compiled stylesheet, and I have no idea how to reorder the mess of spaghetti code that is the MDN SCSS without introducing any more bugs.

Also, https://github.com/mdn/kumascript/pull/983 and https://github.com/mdn/kumascript/pull/963 are relatively self‑contained, but I’ll extract the removal of the now unnecessary {{*_Inline}} invocations into a separate PR.

@schalkneethling I’ve now extracted the duplicate icon fix into https://github.com/mdn/kumascript/pull/1264.

Was this page helpful?
0 / 5 - 0 ratings