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

Change gif, video to .webp format to reduce network payload for meshery page #3978

Closed
wants to merge 7 commits into from

Conversation

ayushthe1
Copy link
Contributor

@ayushthe1 ayushthe1 commented Mar 20, 2023

Description

This PR replaces the gif & video to .webp format as they consume less network payload for meshery page.

This is the network-payload currently :
image

This will be the network payload after merging this PR :
image

Notes for Reviewers
The changes must be verified on safari browser before merging this pr

Signed commits

  • Yes, I signed my commits.

Signed-off-by: ayush_gitk <ayushsharmaa101@gmail.com>
Signed-off-by: ayush_gitk <ayushsharmaa101@gmail.com>
Signed-off-by: ayush_gitk <ayushsharmaa101@gmail.com>
Signed-off-by: ayush_gitk <ayushsharmaa101@gmail.com>
Signed-off-by: ayush_gitk <ayushsharmaa101@gmail.com>
Signed-off-by: ayush_gitk <ayushsharmaa101@gmail.com>
@l5io
Copy link
Contributor

l5io commented Mar 20, 2023

🚀 Preview for commit d593b22 at: https://64183a3ad057c92d70d8addb--layer5.netlify.app

Signed-off-by: ayush_gitk <ayushsharmaa101@gmail.com>
@l5io
Copy link
Contributor

l5io commented Mar 20, 2023

🚀 Preview for commit 0e380c2 at: https://641841ce65f5501fe720438b--layer5.netlify.app

Copy link
Member

@hirentimbadiya hirentimbadiya left a comment

Choose a reason for hiding this comment

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

Overall you converted videos to .webp format from .mp4 but in styling you added header tags but are not used in index.js file , if you can then please make me understand this , otherwise looks good to me.

@@ -47,7 +51,7 @@ const FeaturesSectionWrapper = styled.section`
transform: skew(0deg, 6deg);
padding: 0 1rem 3rem;
text-align: center;
h1, h2, p {
h1, h2, h3, p {
Copy link
Member

Choose a reason for hiding this comment

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

You have added h3 here but haven't used in index.js file

@@ -82,7 +86,7 @@ const FeaturesSectionWrapper = styled.section`
margin: 4rem 0;
align-items: center;
}
h2, h4 {
h2, h4, h3 {
Copy link
Member

Choose a reason for hiding this comment

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

same case here in smp-section added h3 class to add styles but haven't used h3 in index.js file

@@ -242,7 +246,7 @@ const FeaturesSectionWrapper = styled.section`
}

.mesh-mngmnt {
h3 {
h4 {
Copy link
Member

Choose a reason for hiding this comment

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

I am not able to understand it you added h4 here but in index.js file it is still h3

Copy link
Member

Choose a reason for hiding this comment

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

Thanks for pointing this out, @hirentimbadiya 👍

@Chadha93
Copy link
Member

@ayushthe1 Let's discuss this on the Websites call. Please add this as an agenda item in the meeting minutes if you would. :)

@@ -4,7 +4,8 @@ import Button from "../../../reusecore/Button";
import Slider from "react-slick";
import "slick-carousel/slick/slick.css";
import "slick-carousel/slick/slick-theme.css";
import Slide1 from "../images/service mesh performance example.gif";
import MeshVideoWebp from "../images/smp-example.webm";
Copy link
Contributor

Choose a reason for hiding this comment

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

NIT: change the name to MeshVideoWebm

Copy link
Contributor

@Udit-takkar Udit-takkar left a comment

Choose a reason for hiding this comment

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

@ayushthe1 can we also use StaticImage here

<img src={slides_list[i]} loading="lazy" alt={"slide-img" + [i]}/>
?

@Chadha93
Copy link
Member

Merge conflicts... @ayushthe1

@Chadha93
Copy link
Member

@ayushthe1 I'm adding this to the agenda, let's discuss this.

@stale
Copy link

stale bot commented Jun 10, 2023

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the issue/stale Issue has not had any activity for an extended period of time label Jun 10, 2023
@ayushthe1 ayushthe1 closed this Jun 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/stale Issue has not had any activity for an extended period of time type/performance
Development

Successfully merging this pull request may close these issues.

None yet

6 participants