Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

lazy loaded routes for samples #79

Conversation

quentinderoubaix
Copy link
Contributor

No description provided.

@codecov
Copy link

codecov bot commented Aug 9, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (ec0d9b5) 92.07% compared to head (ebb3a7f) 92.07%.

Additional details and impacted files
@@           Coverage Diff           @@
##             main      #79   +/-   ##
=======================================
  Coverage   92.07%   92.07%           
=======================================
  Files          72       72           
  Lines        2081     2081           
  Branches      366      366           
=======================================
  Hits         1916     1916           
  Misses        101      101           
  Partials       64       64           
Flag Coverage Δ
e2e-1 73.86% <ø> (ø)
e2e-2 62.80% <ø> (ø)
e2e-4 73.24% <ø> (ø)
e2e-5 70.42% <ø> (ø)
e2e-7 69.35% <ø> (+8.57%) ⬆️
e2e-8 71.15% <ø> (+0.37%) ⬆️
unit 86.58% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@quentinderoubaix quentinderoubaix marked this pull request as draft August 16, 2023 08:48
@quentinderoubaix quentinderoubaix force-pushed the 46-sample-urls-should-only-import-required-widgets branch from 64e3158 to abe1620 Compare August 16, 2023 12:33
@quentinderoubaix quentinderoubaix changed the title feat: lazy loaded routes for samples [WIP] lazy loaded routes for samples Aug 17, 2023
@quentinderoubaix quentinderoubaix force-pushed the 46-sample-urls-should-only-import-required-widgets branch from abe1620 to 8f55859 Compare November 28, 2023 14:40
@quentinderoubaix quentinderoubaix marked this pull request as ready for review November 28, 2023 14:40
@quentinderoubaix quentinderoubaix changed the title [WIP] lazy loaded routes for samples lazy loaded routes for samples Nov 28, 2023
@quentinderoubaix quentinderoubaix force-pushed the 46-sample-urls-should-only-import-required-widgets branch 5 times, most recently from 8c4a6b7 to 6d4993b Compare December 5, 2023 12:22
@quentinderoubaix quentinderoubaix force-pushed the 46-sample-urls-should-only-import-required-widgets branch from 6d4993b to ebb3a7f Compare December 5, 2023 12:49
let resizeObserver: ResizeObserver | undefined;
let heightSubscription: (UnsubscribeFunction & UnsubscribeObject) | undefined;

const setupObserver = (iframe: HTMLIFrameElement) => {
Copy link
Member

@divdavem divdavem Dec 5, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed, this should be extracted (as part of another PR) in a reactive utility in the core (similar to the intersection utility) that could also be used in the slider.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tracked in #291

Copy link
Member

@divdavem divdavem left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your PR!
Let's merge it to already benefit from the improvements.
We can improve the code of the iframe utility later (to make it more reactive).

@quentinderoubaix quentinderoubaix merged commit fbb421f into AmadeusITGroup:main Dec 5, 2023
13 checks passed
@quentinderoubaix quentinderoubaix deleted the 46-sample-urls-should-only-import-required-widgets branch January 2, 2024 13:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants