Skip to content

Fix/loader performance#5251

Closed
Pankajyadav919 wants to merge 19 commits intosugarlabs:masterfrom
Pankajyadav919:fix/loader-performance
Closed

Fix/loader performance#5251
Pankajyadav919 wants to merge 19 commits intosugarlabs:masterfrom
Pankajyadav919:fix/loader-performance

Conversation

@Pankajyadav919
Copy link
Contributor

###Summary of changes

Fixes undefined language usage by introducing a safe language resolver (getLanguage()).

Prevents potential race conditions by initializing i18n before loading the rest of the application.

Improves language change handling by updating translated content only after i18next is ready.

Cleans up RequireJS configuration by removing unused paths.

###Performance improvements

Avoids unnecessary re-initialization and redundant listeners.

Lays groundwork for enabling proper caching of translation files.

Ensures translated DOM content updates consistently and safely.

###Why this change is needed

The previous implementation could reference an undefined language variable, leading to runtime errors or silent failures.
Additionally, forced cache-busting and repeated DOM queries could negatively impact page load time and performance.

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@github-actions
Copy link
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

loader.test.js

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@github-actions
Copy link
Contributor

✅ All Jest tests passed! This PR is ready to merge.

@github-actions
Copy link
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

loader.test.js

@github-actions
Copy link
Contributor

❌ Some Jest tests failed. Please check the logs and fix the issues before merging.

Failed Tests:

loader.test.js

vanshika2720 pushed a commit to vanshika2720/musicblocks that referenced this pull request Jan 23, 2026
The application was experiencing a 5-second delay before becoming
interactive due to a hardcoded setTimeout(5000) wrapping the critical
initialization logic (setupDependencies() and domReady()).

The domReady! AMD module already ensures the DOM is fully loaded
before executing the callback, making the additional 5-second delay
redundant and harmful to user experience.

Changes:
- Removed setTimeout(5000) wrapper from require(['domReady!']) callback
- setupDependencies() and domReady() now execute immediately when DOM is ready
- All existing Jest tests pass (94 suites, 2532 tests)

Impact:
- UI becomes interactive within 1-2 seconds instead of 5+ seconds
- Improved perceived performance and user experience
- No breaking changes or functionality loss

AI Assistance Disclosure:
This fix was developed with assistance from Claude AI (Gemini/Antigravity).
Full conversation history and prompts are available upon request as per
Sugar Labs AI contribution guidelines.
vanshika2720 pushed a commit to vanshika2720/musicblocks that referenced this pull request Jan 23, 2026
Removed the setTimeout(5000) that was delaying UI initialization.
The domReady callback already ensures DOM is loaded, so the extra
delay was causing poor UX with 5+ second wait times.

Now the app initializes immediately when DOM is ready.
Tested with npm test - all suites pass.
@vanshika2720
Copy link
Contributor

@Pankajyadav919 PR #5298 solves this and merged.So now you can close this !

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