[Demos] YouTube #87

addyosmani · 2019-11-02T16:16:38Z

Description:

There's a neat YouTube demo implemented using React which allows you to see the homepage of videos, the watch page and search page.

I would like for us to make it a little smarter by:

📹 [P0] Video watch page: instead of loading the full embed-video component, we: check if you are on a slow network OR a low-memory device OR have low hardware concurrency and serve down a lighter custom element for the player instead (we may need to wrap it a little for use in React). We can use a similar pattern to network aware code-splitting, but add checks for memory and concurrency in the same boolean check within React.lazy().

🔗 [P0] Routing: Fix routing so that the links to https://marvtron.github.io/watch?v=GKXS_YA9s7E (etc) take you to https://marvtron.github.io/youtube-react/watch?v=GKXS_YA9s7E. Accessing https://marvtron.github.io/youtube-react/watch?v=GKXS_YA9s7E directly should load the video watch page.

🙊 [P1] On slow network or low-memory or low hardware-concurrency, don't load the comments widget and limit (by half) the number of results returned for the recommended videos:

🔧 [P0] I would like us to add the following code to the index so we can quickly show on the watch page which components are important vs not.

<script>
window.debug = () => {
   document.querySelector('.video-container').style.border = '15px solid green';
   document.querySelector('.related-videos').style.border = '15px solid red';
   document.querySelector("#root > div > div.watch-grid > div:nth-child(5)").style.border = '15px solid red';
};
</script>

🎨[P2] Take a quick look at whether render-blocking CSS can be optimized (e.g do we need all of semantic-ui?). I was reading through https://marvtron.github.io/youtube-react/static/css/2.20edb33d.chunk.css and at the very least, I think we don't need to load in the import for the Lato font. YouTube just uses Roboto if installed and I'm cool with us falling back to Arial without loading a Web Font (font-family: roboto, arial, sans-serif; )

I would say this is probably higher priority than ##63 for this week but could be looked at after #86 (#86, #87, #83, then #63).

cc @anton-karlovskiy

The text was updated successfully, but these errors were encountered:

anton-karlovskiy · 2019-11-04T19:27:33Z

@addyosmani

Regarding [P0] Video watch page: instead of loading the full embed-video component, we: check if you are on a slow network OR a low-memory device OR have low hardware concurrency and serve down a lighter custom element for the player instead (we may need to wrap it a little for use in React). We can use a similar pattern to network aware code-splitting, but add checks for memory and concurrency in the same boolean check within React.lazy().

Should we not use hooks for memory and concurrency?
If we use hooks, we might not be able to use network aware code-splitting pattern because I think hooks are not available in React.lazy() async method. So I'd like to suggest that we could use network hook, memory hook, and concurrency hook.

addyosmani · 2019-11-04T20:16:25Z

Ultimately, I want us to use whichever option (hooks or the direct APIs) allows us to avoid sending down the extra code to users. It sounds like we should use hooks and can circle back if we need to rethink this.

If I remember correctly, using the hooks allows us to decide whether to show a heavy or light component but doesn't let us change what bundle gets loaded. Is that correct? We may be okay to use the hooks given the change is "are we using YouTube's iframe embed, that loads in a bunch of its own scripts vs using our own custom lazy-loading embedder?".

anton-karlovskiy · 2019-11-04T20:29:56Z

@addyosmani

Thank you for your clarifying.
I've got your points. As for bundle loading by hooks, I think I should experiment once more as I thought it allows us to change what bundle gets loaded by webpack default configuration of create-react-app.
We can check it soon. I'm wrapping up this feature.

addyosmani · 2019-11-04T20:49:23Z

Awesome. Thanks, Anton!

anton-karlovskiy · 2019-11-05T16:39:55Z

@addyosmani

I've checked the bundle chunks for both lite and heavy mode.

In my opinion, using hooks not only allows us to decide wether to show a heavy or light component but also let us control what bundle gets loaded according to the current deployed project.
So I think there is no significant difference in chunks loading if we just use hooks other than network code-splitting pattern.

Could you please confirm if this is correct?

addyosmani · 2019-11-05T17:43:12Z

That matches my thinking too. Let's move ahead using Hooks here... :)

anton-karlovskiy · 2019-11-06T02:17:37Z

@addyosmani

Because user changing the data-videoid attribute is not supported, dynamically setting the attribute is not working yet.
For example, just clicking on a video in the list does not properly open a video.

I think we should find an solution to it (Paul Irish might implement it soon or we might have to implement it ourselves).

addyosmani · 2019-11-06T14:37:37Z

Ah, I see. This makes sense. Given we won't be clicking on links in the sidebar I think this is OK.

…

On Tue, Nov 5, 2019, 6:17 PM anton-karlovskiy ***@***.***> wrote: @addyosmani <https://github.com/addyosmani> Because user changing the data-videoid attribute <https://github.com/paulirish/lite-youtube-embed/blob/77b70e2279db13ee39aac3d021073687d53c6811/index.html#L152> is not supported, dynamically setting the attribute is not working yet. For example, just clicking on a video in the list does not properly open a video. [image: Capture] <https://user-images.githubusercontent.com/49653735/68262581-bad51100-0054-11ea-9043-5eb1e3800996.PNG> — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#87?email_source=notifications&email_token=AAA3C2JQGTMSCCAOTR2M5W3QSISMFA5CNFSM4JIGOQU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEDFANOQ#issuecomment-550110906>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA3C2OJFN7NIHVYTLNBERTQSISMFANCNFSM4JIGOQUQ> .

anton-karlovskiy · 2019-11-08T10:15:45Z

@addyosmani

[P0] I would like us to add the following code to the index so we can quickly show on the watch page which components are important vs not.

<script>
window.debug = () => {
   document.querySelector('.video-container').style.border = '15px solid green';
   document.querySelector('.related-videos').style.border = '15px solid red';
   document.querySelector("#root > div > div.watch-grid > div:nth-child(5)").style.border = '15px solid red';
};
</script>

It's very weird because the script is loaded and working in local development mode.

But after deployed to firebase, it's not loaded according to Chrome DevTools Elements tab.
Looking into view page source, it's there.

addyosmani · 2019-11-08T21:51:12Z

After taking a look at the latest commits, we could get by without working on the P2 (for now - unless you've started an implementation already?).

The demo as-is is pretty solid. With respect to the window.debug() issue we are running into, my current workflow is to paste the code for this into the DevTools console and just run it. Given this, I don't think it's worth us spending too much time looking at why this wasn't working.

anton-karlovskiy · 2019-11-09T03:13:47Z

@addyosmani

Yes, as we discussed about that alternative with respect to window.debug(), I have not spent much time fixing it.
For P2 as well, I thought it was what we could circle back.

anton-karlovskiy · 2019-11-15T04:59:51Z

@addyosmani

#87 (comment)

FYI: Currently it's working for the version deployed on Firebase. It's weird as it would not work at the beginning.

anton-karlovskiy · 2019-11-15T05:04:12Z

@addyosmani

We just have [P2]. Shall I add this to the backlog and go ahead with this?

addyosmani · 2019-11-15T05:16:03Z

I think it's okay to skip the P2. Perhaps now is a good time to switch to the migration over to the react-adaptive-hooks project while waiting on extra microsite feedback?

anton-karlovskiy · 2019-11-15T05:16:23Z

@addyosmani

This project is missing from the README of https://github.com/GoogleChromeLabs/react-adaptive-hooks.
Is this on purpose?

anton-karlovskiy · 2019-11-15T05:17:42Z

I think it's okay to skip the P2. Perhaps now is a good time to switch to the migration over to the react-adaptive-hooks project while waiting on extra microsite feedback?

It's started. Right now I'm working on updating demos based on react-adaptive-hooks package.

addyosmani · 2019-11-15T05:41:28Z

Missing: nope not on purpose!

…

On Thu, Nov 14, 2019, 9:17 PM Anton Karlovskiy ***@***.***> wrote: I think it's okay to skip the P2. Perhaps now is a good time to switch to the migration over to the react-adaptive-hooks project while waiting on extra microsite feedback? It's started. Right now I'm working on updating demos based on react-adaptive-hooks package. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#87?email_source=notifications&email_token=AAA3C2P7RFT7TQU66XDONADQTYWHNA5CNFSM4JIGOQU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEEEKWMI#issuecomment-554216241>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAA3C2OOO7NOGVBRXOAJKUDQTYWHNANCNFSM4JIGOQUQ> .

addyosmani · 2019-11-17T02:46:23Z

@anton-karlovskiy Would you like to file a PR against the hooks project to add the YouTube app to the list of demos?

anton-karlovskiy · 2019-11-17T08:53:29Z

@addyosmani

No problem. Let me handle.

anton-karlovskiy mentioned this issue Nov 8, 2019

Feature/youtube #89

Merged

addyosmani closed this as completed Nov 26, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Demos] YouTube #87

[Demos] YouTube #87

addyosmani commented Nov 2, 2019 •

edited

anton-karlovskiy commented Nov 4, 2019 •

edited

addyosmani commented Nov 4, 2019

anton-karlovskiy commented Nov 4, 2019

addyosmani commented Nov 4, 2019

anton-karlovskiy commented Nov 5, 2019 •

edited

addyosmani commented Nov 5, 2019

anton-karlovskiy commented Nov 6, 2019 •

edited

addyosmani commented Nov 6, 2019 via email

anton-karlovskiy commented Nov 8, 2019 •

edited

addyosmani commented Nov 8, 2019

anton-karlovskiy commented Nov 9, 2019

anton-karlovskiy commented Nov 15, 2019 •

edited

anton-karlovskiy commented Nov 15, 2019 •

edited

addyosmani commented Nov 15, 2019

anton-karlovskiy commented Nov 15, 2019

anton-karlovskiy commented Nov 15, 2019

addyosmani commented Nov 15, 2019 via email

addyosmani commented Nov 17, 2019

anton-karlovskiy commented Nov 17, 2019

[Demos] YouTube #87

[Demos] YouTube #87

Comments

addyosmani commented Nov 2, 2019 • edited

anton-karlovskiy commented Nov 4, 2019 • edited

addyosmani commented Nov 4, 2019

anton-karlovskiy commented Nov 4, 2019

addyosmani commented Nov 4, 2019

anton-karlovskiy commented Nov 5, 2019 • edited

addyosmani commented Nov 5, 2019

anton-karlovskiy commented Nov 6, 2019 • edited

addyosmani commented Nov 6, 2019 via email

anton-karlovskiy commented Nov 8, 2019 • edited

addyosmani commented Nov 8, 2019

anton-karlovskiy commented Nov 9, 2019

anton-karlovskiy commented Nov 15, 2019 • edited

anton-karlovskiy commented Nov 15, 2019 • edited

addyosmani commented Nov 15, 2019

anton-karlovskiy commented Nov 15, 2019

anton-karlovskiy commented Nov 15, 2019

addyosmani commented Nov 15, 2019 via email

addyosmani commented Nov 17, 2019

anton-karlovskiy commented Nov 17, 2019

addyosmani commented Nov 2, 2019 •

edited

anton-karlovskiy commented Nov 4, 2019 •

edited

anton-karlovskiy commented Nov 5, 2019 •

edited

anton-karlovskiy commented Nov 6, 2019 •

edited

anton-karlovskiy commented Nov 8, 2019 •

edited

anton-karlovskiy commented Nov 15, 2019 •

edited

anton-karlovskiy commented Nov 15, 2019 •

edited