Netcentric / vg-macktrucks-com-rd

Franklin Site Redesign for https://www.macktrucks.com
Apache License 2.0
1 stars 12 forks source link

Images grid carousel #38 #165

Closed TomaszDziezykNetcentric closed 1 year ago

TomaszDziezykNetcentric commented 1 year ago

Fix #38

Test URLs:

aem-code-sync[bot] commented 1 year ago

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.

aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
TomaszDziezykNetcentric commented 1 year ago

I noticed some odd scroll behaviour when navigating in the preview list. It’s also present on desktop, but due to the small viewport on mobile, it’s more distracting: https://github.com/Netcentric/vg-macktrucks-com-rd/assets/133873665/52bcc85f-4c01-4dce-aa8d-71ff51042180

Fixed

aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
taimurCognizant commented 1 year ago

@cogniSyb @TomaszDziezykNetcentric @Lakshmishri I also have a generic comment on modal open/close transition. It is very slow, taking 1 second. And since the transition is applied on the height property I can see the model expanding. Moreover, Adding transition to height property is heavy on performance and on slower device the animation would be choppy. Not sure if that is expected behavior or not. If we have freedom to apply any kind of transition I would suggest to transition only the opacity property

aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI
TomaszDziezykNetcentric commented 1 year ago

Screenshot 2023-11-15 at 20 54 14 I think the space should be less . @cogniSyb - wdyt ?

I was testing it on smaller devices... I agree that it looks weird now :( FIxed

TomaszDziezykNetcentric commented 1 year ago

@cogniSyb @TomaszDziezykNetcentric @Lakshmishri I also have a generic comment on modal open/close transition. It is very slow, taking 1 second. And since the transition is applied on the height property I can see the model expanding. Moreover, Adding transition to height property is heavy on performance and on slower device the animation would be choppy. Not sure if that is expected behavior or not. If we have freedom to apply any kind of transition I would suggest to transition only the opacity property

We can update the animation, but I would like to do this in the scope of the other issue :) The change will be very easy, but because we are using this modal at other paces that are out of the scope of the redesign it would be better to test it in a separate issue scope.

aem-code-sync[bot] commented 1 year ago
Page Scores Audits Google
/drafts/tdziezyk/v2/images-grid-with-modal PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI