fix: [AXIMST-719] Course unit - Xblock problems #213

PKulkoRaccoonGang · 2024-03-26T11:15:26Z

YouTrack tickets:

monteri · 2024-03-28T13:29:23Z

src/course-unit/course-xblock/xblock-content/iframe-wrapper/iframe-wrapper.js

  const resources = normalizeResources(sourceResources);

  /* Extract CSS resources. */
  const cssUrls = filterAndExtractResources(resources, 'url', 'text/css');
  const sheets = filterAndExtractResources(resources, 'text', 'text/css');

+  let additionalCSSTags = '';


Suggested change

let additionalCSSTags = '';

let additionalCssTags = '';

[nit] I remember on the Paragon we had a linter that didn't like when we did something like CSS in the middle of a variable name. The idea is to follow camelCase when it is possible. When we do CSS it is kind of small camelCase violation.

Ok, corrected

monteri · 2024-03-28T13:39:22Z

src/course-unit/course-xblock/xblock-content/iframe-wrapper/iframe-wrapper.js

+  additionalCSSTags += stylesWithContent
+    .map(sheet => {
+      if (Array.isArray(sheet)) {
+        return sheet.map(subSheet => `<style>${subSheet}</style>`).join('\n');
+      }
+      return `<style>${sheet}</style>`;
+    })
+    .join('\n');


Suggested change

additionalCSSTags += stylesWithContent

.map(sheet => {

if (Array.isArray(sheet)) {

return sheet.map(subSheet => `<style>${subSheet}</style>`).join('\n');

}

return `<style>${sheet}</style>`;

})

.join('\n');

const additionalCSSTags = stylesWithContent.flatMap(sheet => `<style>${sheet}</style>`).join('\n');

In JavaScript, strings are immutable. Every time you use += or concatenation, a new string is created in memory, and the old one is potentially discarded. This can lead to excessive memory usage if you do it often in a loop.

By the way flatMap can help with nested arrays to make it flat and map it at the same time.

Yeah, good catch! flatMap is very good to apply here!

monteri · 2024-03-28T13:48:42Z

src/course-unit/course-xblock/xblock-content/iframe-wrapper/iframe-wrapper.js

+  modifiedHtml = modifiedHtml.replaceAll('url(&#39;/asset', `url('${studioBaseUrl}/asset`);
+  modifiedHtml = modifiedHtml.replaceAll('src="/asset', `src="${studioBaseUrl}/asset`);
+  modifiedHtml = modifiedHtml.replaceAll('src=&#34;/asset', `src=&#34;${studioBaseUrl}/asset`);
+  modifiedHtml = modifiedHtml.replaceAll('href="/asset', `href="${studioBaseUrl}/asset`);
+  modifiedHtml = modifiedHtml.replaceAll('src=&#34;/static', `src=&#34;${studioBaseUrl}/static`);
+  modifiedHtml = modifiedHtml.replaceAll('src="/static', `src="${studioBaseUrl}/static`);
+  modifiedHtml = modifiedHtml.replaceAll('data-target="/preview', `data-target="${studioBaseUrl}/preview`);
+  modifiedHtml = modifiedHtml.replaceAll('data-url="/preview', `data-url="${studioBaseUrl}/preview`);
+  modifiedHtml = modifiedHtml.replaceAll('src="/preview', `src="${studioBaseUrl}/preview`);
+  modifiedHtml = modifiedHtml.replaceAll('src="/media', `src="${studioBaseUrl}/media`);
+  modifiedHtml = modifiedHtml.replaceAll(': "/asset', `: "${studioBaseUrl}/asset`);
+  modifiedHtml = modifiedHtml.replaceAll(': "/xblock', `: "${studioBaseUrl}/xblock`);


Since it is growing in size we can make an object with key as a first argument of replaceAll and value as a second. Put it somewhere before the string HTML string and do something like

// Block that replaces relative urls with absolute urls Object.entries(relativeToAbosluteXBlocksUrls).forEach(([key, value]) => { modifiedHtml = modifiedHtml.replaceAll(key, value); });

codecov-commenter · 2024-03-28T15:56:09Z

Codecov Report

Attention: Patch coverage is 10.52632% with 17 lines in your changes are missing coverage. Please review.

❗ No coverage uploaded for pull request base (ts-develop@6509cc7). Click here to learn what that means.

Files	Patch %	Lines
src/course-unit/course-xblock/utils.js	0.00%	7 Missing ⚠️
...ck/xblock-content/iframe-wrapper/iframe-wrapper.js	0.00%	6 Missing ⚠️
src/course-unit/course-xblock/CourseXBlock.jsx	33.33%	2 Missing ⚠️
...nit/course-xblock/xblock-content/XBlockContent.jsx	0.00%	1 Missing ⚠️
src/course-unit/data/api.js	0.00%	1 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files

@@              Coverage Diff              @@
##             ts-develop     #213   +/-   ##
=============================================
  Coverage              ?   88.38%           
=============================================
  Files                 ?      689           
  Lines                 ?    11894           
  Branches              ?     2539           
=============================================
  Hits                  ?    10512           
  Misses                ?     1322           
  Partials              ?       60

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

* fix: [AXIMST-719] fixed xblock problems * fix: added asset and static replacer * feat: added style extractor * refactor: code refactoring * refactor: after review

fix: [AXIMST-27] Unit page - change iframe pararm for display xblock content (#151) feat: [AXIMST-52] display unit and xblock render errors (#191) * feat: [AXIMST-52] display unit and xblock render errors * fix: after discussion * fix: after review feat: Course unit - Rendering xblocks in iframes (#201) * feat: added xblock render engine * feat: connected iframe template * feat: CSS and JS transfering * feat: show video xblock * feat: added multiply xblocks * feat: rendering advanced components * fix: add solution for request from within the iframe * feat: iframe height * refactor: code refactoring * fix: fixed platform ajax prefix * refactor: global refactoring * refactor: show LTI xblock * refactor: removed old xblock-content * refactor: code refactoring * refactor: some refactoring * refactor: refactoring * refactor: code refactoring * refactor: refactoring after review --------- Co-authored-by: monteri <lansevermore> feat: [AXIMST-739] added xblock actions (#218) * feat: [AXIMST-739] added xblock actions * refactor: code refactoring * refactor: refactoring after review fix: after cherru-pick fix: after cherry-pick refactor: removed canEdit mock (#224) fix: [AXIMST-655] fixed position of the view-port after loading the unit page (#217) fix: [AXIMST-719] Course unit - Xblock problems (#213) * fix: [AXIMST-719] fixed xblock problems * fix: added asset and static replacer * feat: added style extractor * refactor: code refactoring * refactor: after review fix: [AXIMST-718] Course unit - Created logic for getting CSRF token (#226) * fix: created logic for getting csrf token * fix: usage of csrfTokenData * refactor: code refactoring --------- Co-authored-by: monteri <lansevermore> fix: [AXIMST-745] added studioBaseUrl for static paths (#228) refactor: [AXIMST-746] Unit page performance refactoring (#229) Co-authored-by: Kyrylo Hudym-Levkovych <[email protected]> fix: [AXIMST-785] fixed discard logic (#232) feat: [AXIMST-800] Course unit - Added Collapse and Expand all buttons for xblocks (#234) * feat: [AXIMST-800] added Collapse and Expand all buttons for xblocks * feat: added tests refactor: code refactoring

* fix: [AXIMST-719] fixed xblock problems * fix: added asset and static replacer * feat: added style extractor * refactor: code refactoring * refactor: after review

PKulkoRaccoonGang changed the title ~~Peter kulko/xblock problems~~ DRAFT: [AXIMST-719] Course unit - Xblock problems Mar 26, 2024

PKulkoRaccoonGang self-assigned this Mar 26, 2024

PKulkoRaccoonGang added the course-unit label Mar 26, 2024

PKulkoRaccoonGang marked this pull request as draft March 26, 2024 11:16

PKulkoRaccoonGang force-pushed the Peter_Kulko/xblock-problems branch 2 times, most recently from ff9c1dd to 249cd15 Compare March 28, 2024 11:17

PKulkoRaccoonGang requested a review from monteri March 28, 2024 11:20

PKulkoRaccoonGang marked this pull request as ready for review March 28, 2024 11:20

PKulkoRaccoonGang added 3 commits March 28, 2024 13:21

fix: [AXIMST-719] fixed xblock problems

6f460dd

fix: added asset and static replacer

ea82d85

feat: added style extractor

dcbe895

PKulkoRaccoonGang force-pushed the Peter_Kulko/xblock-problems branch from 249cd15 to 3aba303 Compare March 28, 2024 11:22

refactor: code refactoring

4aee493

PKulkoRaccoonGang force-pushed the Peter_Kulko/xblock-problems branch from 3aba303 to 4aee493 Compare March 28, 2024 11:38

PKulkoRaccoonGang changed the title ~~DRAFT: [AXIMST-719] Course unit - Xblock problems~~ fix: [AXIMST-719] Course unit - Xblock problems Mar 28, 2024

monteri requested changes Mar 28, 2024

View reviewed changes

refactor: after review

a044959

PKulkoRaccoonGang requested a review from monteri March 28, 2024 15:54

monteri approved these changes Mar 29, 2024

View reviewed changes

monteri merged commit 0a2bb22 into ts-develop Mar 29, 2024
3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: [AXIMST-719] Course unit - Xblock problems #213

fix: [AXIMST-719] Course unit - Xblock problems #213

PKulkoRaccoonGang commented Mar 26, 2024 •

edited

Loading

monteri Mar 28, 2024

PKulkoRaccoonGang Mar 28, 2024

monteri Mar 28, 2024

PKulkoRaccoonGang Mar 28, 2024

monteri Mar 28, 2024

PKulkoRaccoonGang Mar 28, 2024

codecov-commenter commented Mar 28, 2024

fix: [AXIMST-719] Course unit - Xblock problems #213

fix: [AXIMST-719] Course unit - Xblock problems #213

Conversation

PKulkoRaccoonGang commented Mar 26, 2024 • edited Loading

monteri Mar 28, 2024

Choose a reason for hiding this comment

PKulkoRaccoonGang Mar 28, 2024

Choose a reason for hiding this comment

monteri Mar 28, 2024

Choose a reason for hiding this comment

PKulkoRaccoonGang Mar 28, 2024

Choose a reason for hiding this comment

monteri Mar 28, 2024

Choose a reason for hiding this comment

PKulkoRaccoonGang Mar 28, 2024

Choose a reason for hiding this comment

codecov-commenter commented Mar 28, 2024

Codecov Report

PKulkoRaccoonGang commented Mar 26, 2024 •

edited

Loading