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

[DO NOT MERGE] Move some script elements (demo) #1687

Closed
wants to merge 2 commits into from

Conversation

riyadh-h
Copy link
Contributor

@riyadh-h riyadh-h commented Nov 9, 2021

These script elements found in Page.njk have been moved down to be just above the body closing tag:

<script src="{{ asset.vue }}"></script>
<script src="{{ asset.markBindJs }}"></script>
<script src="{{ asset.pageVueRenderJs }}"></script>
<script src="{{ asset.jQuery }}"></script>

There are two commits: one for where the script elements are down below, and one for where they're up above. You can check-out between these commits to see the changes without manually modifying the code.

Testing instructions:

  1. (optional) Create a new MarkBind project.
  2. Check-out the commit that you want to test.
  3. Serve the MarkBind project.
  • Note: the following steps assume the use of Edge/Chrome; the same (or similar) instructions might be applicable to other browsers.
  1. Go to the MarkBind serving address (if it isn't open already).
  2. Right-click anywhere on the page and select Inspect.
  3. Click on the Network tab, then turn on Disabling Cache and set the throttling option to Slow 3G.
  4. Refresh the page; once the page is fully-loaded, a timestamp with the label Load will appear on the bottom of Network tab.
    • If the scripts are at the bottom during this step, then the header padding FOUC should take more time to happen.
  5. Repeat Step 6 if more tests are required for the current commit.
  6. Once finished, stop the MarkBind serving process.
  7. Repeat Steps 1-8 for the other commit if necessary.

@ang-zeyu
Copy link
Contributor

Thanks for the instructions @riyadh-h

Some delay to header padding FOUC is to be expected (relative to the first content shown), since we're doing dynamic style detection here, and the scripts are now downloaded and run later. I think its an ok tradeoff, especially considering the underlying issue with the headers can be fixed separately.

As for the other forms of FOUC, our stylesheets are kicking in later as they are injected by this webpack plugin during development (-d cli option for hot reload of styles). Other builds (e.g. netlify) shouldn't have this issue as the css is extracted and bundled.

If you are up for it, the latter plugin now supports hot module replacement as well, I think we could experiment with that to fix this for -d. But otherwise we can fix it separately later on too (not high priority since its a developer issue); We can focus on whether everything still functions correctly for now (if anything in the usingComponents page breaks in particular).

@ang-zeyu
Copy link
Contributor

Closing this as its been stale for a while, but this is something we'd very much like to have done still.

@ang-zeyu ang-zeyu closed this Dec 17, 2022
@LamJiuFong LamJiuFong mentioned this pull request Apr 3, 2024
14 tasks
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