feat(macros/EmbedInteractiveExample): use height from height-data.json #7679

NiedziolkaMichal · 2022-11-26T22:56:47Z

This is the second part of PR mdn/bob#839 in BOB, which makes height-data.json be created while building interactive examples. That file contains the height of every interactive example, so that height no longer needs to be provided from content repository and we can also remove the height of each interactive editor type from here.

The height of the editor is now calculated by JS code in interactive-examples.tsx. During build, iframe of example is created, together with data-heights attribute, which contains information about the height of the editor. This is an exemplary value:

[
   {
      "minFrameWidth": 0,
      "height": 723
   },
   {
      "minFrameWidth": 590,
      "height": 654
   }
]

The component will use the last height, for which the value of minFrameWidth was greater or equal to the calculated width of the iframe. This works just like media queries, but we don't use the width of the window in here, which gives incorrect results because of interference from the rest of the UI.

I changed the key of the Prose component to the following because the paragraph under iframe was causing duplicated react key exception(both having null value).

key={section.value.id || `top_level_prose${i}`}

If this gets merged, I will make the next PR to remove the second argument(height class) from every usage of EmbedInteractiveExample in the content repository.

This PR depends on mdn/bob#839 and shouldn't be merged until BOB update is deployed.
Tests are failing because iframe cannot be shown, until #839 is merged

caugner

Thank you, just some nits (moving complexity out of the macro, add types).

caugner · 2022-11-28T19:45:32Z

kumascript/macros/EmbedInteractiveExample.ejs

+const heightData = await mdn.fetchInteractiveExampleHeightData();

-if ($0.includes('/js/')) {
-  heightClass = 'is-js-height';
-}
-
-if ($1) {
-  let supportedHeights = ['shorter', 'taller', 'tabbed-shorter', 'tabbed-standard', 'tabbed-taller'];
-  let heightIsSupported = (supportedHeights.indexOf($1) > -1);
-
-  if (heightIsSupported) {
-    heightClass = 'is-' + $1 + '-height';
-  } else {
-    throw new Error(`An unrecognized second size parameter to EmbedInteractiveExample ('${$1}')`);
-  }
+let height;
+if (heightData[$0]) {
+  height = heightData[$0];
+} else if ($0.includes('/js/')) {
+  height = 'js';
+} else {
+  height = 'default';
 }
+const heightClass = 'is-' + height + '-height';


Let's extract this whole section as mdn.getInteractiveExampleHeightClass(path), so that this macro knows nothing about where the height comes from.

Also, let's keep the $1 parameter for now and pass it to this method, to use as a fallback as long as the height-data.json is not available.

Once the height-data.json is available, we can deprecate the 2nd parameter.

I have moved the whole section to getInteractiveExampleHeight. Thanks

Since there was no reply for a long time, I have moved the exact height information to height-data.json. Old classes are no longer compatible, so I didn't apply the second suggestion.

caugner · 2022-11-28T19:46:36Z

kumascript/src/api/mdn.ts


 // Module level caching for repeat calls to fetchWebExtExamples().
 let webExtExamples: any = null;

+let interactiveExampleHeightData: object = null;


Suggested change

let interactiveExampleHeightData: object = null;

type InteractiveExamplesHeight = 'shorter' | 'taller' | 'tabbed-shorter' | 'tabbed-standard' | 'tabbed-taller';

type InteractiveExamplesHeightData = Record<string, InteractiveExamplesHeight>;

let interactiveExampleHeightData: InteractiveExamplesHeightData = null;

I added the correct type instead of the object. I have moved it to interactive-example.tsx though, so the whole logic is located in a single place. In the future I will port BOB to TS, so we can remove the types from here.

caugner · 2022-11-28T19:49:01Z

kumascript/src/api/mdn.ts

+  async fetchInteractiveExampleHeightData() {
+    if (!interactiveExampleHeightData) {
+      try {
+        interactiveExampleHeightData = await got(


Suggested change

interactiveExampleHeightData = await got(

interactiveExampleHeightData = await got<InteractiveExamplesHeightData>(

Added, thank you

NiedziolkaMichal · 2022-11-28T21:33:56Z

@caugner All of that sounds good, I will add a commit with those changes later.

I realized now that there is a better way to tackle this problem. Currently, heights are hardcoded in interactive-examples.scss in YARI and distributed across multiple CSS files in BOB. I could make height-data.json send height in pixels instead of classes. A single record would look like that:

    "pages/tabbed/audio.html": {
        "landscape-height": "421px",
        "portrait-height": "548px"
    }

The file would get a bit bigger, but it would allow more complex height adjustments. For JavaScript examples, we could make height calculated by the formula BASE_HEIGHT + amountOfLines * LINE_HEIGHT. It would make examples like Optional Chaining not be half empty.
Would you agree to that?
Edit: Since there was no reply for a long time, I have gone along with this, but JSON looks a bit different. I have updated the first post with the exact details.

github-actions · 2022-12-16T09:50:37Z

This pull request has merge conflicts that must be resolved before it can be merged.

schalkneethling · 2023-01-16T09:43:51Z

Hey @NiedziolkaMichal, a friendly ping. This one has a conflict that needs to be resolved. Thanks!

NiedziolkaMichal · 2023-01-18T23:41:19Z

@schalkneethling It's done :) Tests seem to be failing because height-data.json is not yet available.

github-actions · 2023-01-23T14:36:24Z

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions · 2023-02-08T12:29:18Z

This pull request has merge conflicts that must be resolved before it can be merged.

NiedziolkaMichal · 2023-02-25T13:50:37Z

I have rechecked everything and the code is working fine, but https://interactive-examples.mdn.mozilla.net/height-data.json is still returning 404. While building interactive examples, this file exists and is available under the right location, but for some reason, it is not present in the production. I think that maybe "heightData": "./docs/height-data.json" line is missing in the .bobconfigrc in the production environment.

caugner · 2023-03-08T13:48:18Z

Hi @NiedziolkaMichal, I wonder if we couldn't just fix yari to listen to this height event:

https://github.com/mdn/bob/blob/077d0203a9c45b2a143a734169710c4ab49b4ce9/editor/js/editor-libs/events.js#L85-L92

NiedziolkaMichal · 2023-03-08T13:55:19Z

I was thinking about it, but there is a significant drawback to this. Individual examples usually load 2-3 seconds after the rest of the content on my PC, where I have high-speed internet. If we send height information through an event, we will have a significant reflow after those few seconds, which would annoy the users.
I was planning to use the event for examples that are not supported by the user's device because there is no way to predict this during the build, while currently a lot of space is being wasted.

github-actions · 2023-03-15T21:19:33Z

This pull request has merge conflicts that must be resolved before it can be merged.

github-actions · 2024-03-13T12:53:59Z

This pull request has merge conflicts that must be resolved before it can be merged.

fiji-flo · 2024-03-28T15:04:42Z

Sorry for not addressing this in time. I see the issue, but we don't want to set height in JS here. We'll address this later this year.

github-actions bot added the macros tracking issues related to kumascript macros label Nov 26, 2022

This was referenced Nov 26, 2022

Adds height-data.json mdn/bob#839

Merged

style: Enable ESLint and Prettier for all files mdn/interactive-examples#2288

Closed

caugner requested changes Nov 28, 2022

View reviewed changes

caugner marked this pull request as draft November 28, 2022 19:51

caugner changed the title ~~Interactive Examples use height from height-data.json~~ feat(macros/EmbedInteractiveExample): use height from height-data.json Nov 28, 2022

caugner mentioned this pull request Nov 28, 2022

Can we make macro iframes auto-size? #4240

Open

github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Dec 16, 2022

schalkneethling requested a review from fiji-flo January 16, 2023 09:44

NiedziolkaMichal force-pushed the height-data branch from f711daf to 363f73f Compare January 18, 2023 23:03

github-actions bot removed the merge conflicts 🚧 Please rebase onto or merge the latest main. label Jan 18, 2023

NiedziolkaMichal requested review from caugner and removed request for fiji-flo January 18, 2023 23:42

NiedziolkaMichal marked this pull request as ready for review January 18, 2023 23:42

github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Jan 23, 2023

github-actions bot removed the merge conflicts 🚧 Please rebase onto or merge the latest main. label Jan 23, 2023

github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Feb 8, 2023

NiedziolkaMichal added 5 commits February 24, 2023 20:37

Interactive Examples use height from height-data.json

7ba3075

Use height of editor from height-data.json

7a10178

Move logic to getInteractiveExampleHeight

75bb59e

Applied github lint suggestions

f0f4200

Finish rebase

d00560a

NiedziolkaMichal force-pushed the height-data branch from c2d7082 to d00560a Compare February 25, 2023 13:36

github-actions bot removed the merge conflicts 🚧 Please rebase onto or merge the latest main. label Feb 25, 2023

lint

8da0c82

NiedziolkaMichal mentioned this pull request Mar 12, 2023

feat(interactive-examples): measure actions #8337

Merged

github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Mar 15, 2023

caugner removed their request for review May 2, 2023 08:19

bsmth requested a review from LeoMcA May 4, 2023 14:26

github-actions bot added the idle label Jun 3, 2023

github-actions bot removed the idle label Jun 20, 2023

github-actions bot added the idle label Jul 20, 2023

Merge branch 'main' into height-data

15bb65f

NiedziolkaMichal requested a review from a team as a code owner January 2, 2024 10:47

github-actions bot removed merge conflicts 🚧 Please rebase onto or merge the latest main. idle labels Jan 2, 2024

github-actions bot added the idle label Feb 2, 2024

github-actions bot added the merge conflicts 🚧 Please rebase onto or merge the latest main. label Mar 13, 2024

github-actions bot removed the idle label Mar 13, 2024

fiji-flo closed this Mar 28, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(macros/EmbedInteractiveExample): use height from height-data.json #7679

feat(macros/EmbedInteractiveExample): use height from height-data.json #7679

NiedziolkaMichal commented Nov 26, 2022 •

edited

Loading

caugner left a comment

caugner Nov 28, 2022

NiedziolkaMichal Jan 18, 2023

caugner Nov 28, 2022

NiedziolkaMichal Jan 18, 2023

caugner Nov 28, 2022

NiedziolkaMichal Jan 18, 2023

NiedziolkaMichal commented Nov 28, 2022 •

edited

Loading

github-actions bot commented Dec 16, 2022

schalkneethling commented Jan 16, 2023

NiedziolkaMichal commented Jan 18, 2023

github-actions bot commented Jan 23, 2023

github-actions bot commented Feb 8, 2023

NiedziolkaMichal commented Feb 25, 2023

caugner commented Mar 8, 2023

NiedziolkaMichal commented Mar 8, 2023

github-actions bot commented Mar 15, 2023

github-actions bot commented Mar 13, 2024

fiji-flo commented Mar 28, 2024

-let interactiveExampleHeightData: object = null;
+type InteractiveExamplesHeight = 'shorter' | 'taller' | 'tabbed-shorter' | 'tabbed-standard' | 'tabbed-taller';
+type InteractiveExamplesHeightData = Record<string, InteractiveExamplesHeight>;
+let interactiveExampleHeightData: InteractiveExamplesHeightData = null;

	interactiveExampleHeightData = await got(
	interactiveExampleHeightData = await got<InteractiveExamplesHeightData>(

feat(macros/EmbedInteractiveExample): use height from height-data.json #7679

feat(macros/EmbedInteractiveExample): use height from height-data.json #7679

Conversation

NiedziolkaMichal commented Nov 26, 2022 • edited Loading

caugner left a comment

Choose a reason for hiding this comment

caugner Nov 28, 2022

Choose a reason for hiding this comment

NiedziolkaMichal Jan 18, 2023

Choose a reason for hiding this comment

caugner Nov 28, 2022

Choose a reason for hiding this comment

NiedziolkaMichal Jan 18, 2023

Choose a reason for hiding this comment

caugner Nov 28, 2022

Choose a reason for hiding this comment

NiedziolkaMichal Jan 18, 2023

Choose a reason for hiding this comment

NiedziolkaMichal commented Nov 28, 2022 • edited Loading

github-actions bot commented Dec 16, 2022

schalkneethling commented Jan 16, 2023

NiedziolkaMichal commented Jan 18, 2023

github-actions bot commented Jan 23, 2023

github-actions bot commented Feb 8, 2023

NiedziolkaMichal commented Feb 25, 2023

caugner commented Mar 8, 2023

NiedziolkaMichal commented Mar 8, 2023

github-actions bot commented Mar 15, 2023

github-actions bot commented Mar 13, 2024

fiji-flo commented Mar 28, 2024

NiedziolkaMichal commented Nov 26, 2022 •

edited

Loading

NiedziolkaMichal commented Nov 28, 2022 •

edited

Loading