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

Truck features #43 #155

Merged
merged 24 commits into from
Nov 17, 2023
Merged

Truck features #43 #155

merged 24 commits into from
Nov 17, 2023

Conversation

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Oct 30, 2023

Hello, I'm the AEM Code Sync Bot and I will run some test suites that validate the page speed.
In case there are problems, just click the checkbox below to rerun the respective action.

  • Re-run PSI Checks

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Oct 30, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link
Contributor

aem-code-sync bot commented Oct 30, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@TomaszDziezykNetcentric TomaszDziezykNetcentric marked this pull request as ready for review October 30, 2023 14:39
@TomaszDziezykNetcentric TomaszDziezykNetcentric changed the base branch from main to develop October 30, 2023 14:41
@cogniSyb
Copy link
Collaborator

I expect the main browser scrollbar to be present, and that as a user I can also use the keyboard (arrows/spacebar/page up/page down).

Example of this can be found on https://www.elgato.com/eu/en/p/prompter:
Screenshot 2023-10-31 at 09 51 04

@cogniSyb cogniSyb changed the title 43 truck features Truck features #43 Oct 31, 2023
Copy link
Contributor

aem-code-sync bot commented Nov 6, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Contributor

aem-code-sync bot commented Nov 6, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@TomaszDziezykNetcentric
Copy link
Collaborator Author

I expect the main browser scrollbar to be present, and that as a user I can also use the keyboard (arrows/spacebar/page up/page down).

Example of this can be found on https://www.elgato.com/eu/en/p/prompter: Screenshot 2023-10-31 at 09 51 04

Updated

Copy link
Contributor

aem-code-sync bot commented Nov 6, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Contributor

aem-code-sync bot commented Nov 14, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@TomaszDziezykNetcentric
Copy link
Collaborator Author

TomaszDziezykNetcentric commented Nov 14, 2023

UX-wise I’m expecting to switch faster to another slide. I think that right now we’re scrolling past the whole image before switching. Could we make it more 'responsive' to the user scroll/keyboard action?

I made the changes. Now we can change the number of pixels needed to change the slide in one variable (SLIDE_SCROLL_PADDING_IN_PX).

@taimurCognizant
Copy link
Collaborator

taimurCognizant commented Nov 15, 2023

image

@TomaszDziezykNetcentric In mobile view there a is huge space after the bottom text. As per my understanding of the requirements and design the bottom text should stick to the bottom of the mobile screen and the images should be in the middle of the heading and the text
@cogniSyb do you have any comments on this?

@taimurCognizant
Copy link
Collaborator

@TomaszDziezykNetcentric also the blur effect is missing for mobile. Not sure if that was part of requirement or not

Copy link
Contributor

aem-code-sync bot commented Nov 15, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Contributor

aem-code-sync bot commented Nov 15, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Contributor

aem-code-sync bot commented Nov 16, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@cogniSyb
Copy link
Collaborator

@TomaszDziezykNetcentric In mobile view there a is huge space after the bottom text. As per my understanding of the requirements and design the bottom text should stick to the bottom of the mobile screen and the images should be in the middle of the heading and the text @cogniSyb do you have any comments on this?

Good point, @taimurCognizant. This is a bit difficult to get out of Figma. We might want to use justify-content: space-evenly; or justify-content: center; here for mobile.

@TomaszDziezykNetcentric
Copy link
Collaborator Author

@TomaszDziezykNetcentric also the blur effect is missing for mobile. Not sure if that was part of requirement or not

I agree that the blur effect should be there.
Code updated

Copy link
Contributor

aem-code-sync bot commented Nov 17, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@TomaszDziezykNetcentric
Copy link
Collaborator Author

@TomaszDziezykNetcentric In mobile view there a is huge space after the bottom text. As per my understanding of the requirements and design the bottom text should stick to the bottom of the mobile screen and the images should be in the middle of the heading and the text @cogniSyb do you have any comments on this?

Good point, @taimurCognizant. This is a bit difficult to get out of Figma. We might want to use justify-content: space-evenly; or justify-content: center; here for mobile.

@cogniSyb @taimurCognizant
The aligments is updated
obraz

Copy link
Contributor

aem-code-sync bot commented Nov 17, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Contributor

aem-code-sync bot commented Nov 17, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link
Contributor

aem-code-sync bot commented Nov 17, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI


let slideIndex = 0;

window.addEventListener('scroll', debounce(() => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

@TomaszDziezykNetcentric The snapping function doesn't work properly. Because of debounce the values inside the function are not precise. I suggest removing debouncing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

debouce removed

Copy link
Contributor

aem-code-sync bot commented Nov 17, 2023

Page Scores Audits Google
/drafts/tdziezyk/v2/truck-features PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@cogniSyb cogniSyb merged commit 97cd568 into develop Nov 17, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FR Functional requirement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Truck features
5 participants