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

feat(thought-leadership): add URL/query parameter to sidebar filters #279

Conversation

sahmad-merative
Copy link

@sahmad-merative sahmad-merative commented Jul 14, 2023

Issue

Fixes #

Description

Added deep linking within each filtering within the Thought Leadership landing
filters selected are added to the url query params and when link is directly used, thought leadership page loads withfilters pre-selected
New

Changed

Removed

Design Specs

If applicable, add the direct link to the design specs of the component/feature that's part of this PR.

Test URLs

Before (Changes from feat/thought-leadership): https://feat-thought-leadership--merative2--hlxsites.hlx.page/thought-leadership
After (Changes from this PR): https://753-query-param-sidebar--merative2--sahmad-merative.hlx.page/thought-leadership

Testing Instruction

If applicable, please describe the tests that you ran to verify your changes. Provide instructions and link to the hlx deploy preview so that QA and the design team can provide proper testing.

image

@sahmad-merative sahmad-merative added Thought Leadership Used for work related to Thought Leadership On-Hold Work is on-hold until proper sign off labels Jul 14, 2023
@aem-code-sync
Copy link

aem-code-sync bot commented Jul 14, 2023

Hello, I'm Franklin 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

aem-code-sync bot commented Jul 14, 2023

Page Scores Audits Google
/thought-leadership Lighthouse returned error: NO_FCP. The page did not paint any content. Please ensure you keep the browser window in the foreground during the load and try again. (NO_FCP) PSI

@proeung
Copy link

proeung commented Jul 20, 2023

@sahmad-merative #286 has been merged into the feature branch. Please rebase or open a new PR that includes the changes for exposing the query parameter for the filtering items.

@aem-code-sync
Copy link

aem-code-sync bot commented Jul 21, 2023

Page Scores Audits Google
/thought-leadership Lighthouse returned error: NO_FCP. The page did not paint any content. Please ensure you keep the browser window in the foreground during the load and try again. (NO_FCP) PSI

@aem-code-sync
Copy link

aem-code-sync bot commented Jul 21, 2023

Page Scores Audits Google
/thought-leadership Lighthouse returned error: NO_FCP. The page did not paint any content. Please ensure you keep the browser window in the foreground during the load and try again. (NO_FCP) PSI

@sahmad-merative sahmad-merative removed the On-Hold Work is on-hold until proper sign off label Jul 21, 2023
@aem-code-sync
Copy link

aem-code-sync bot commented Jul 21, 2023

Page Scores Audits Google
/thought-leadership Lighthouse returned error: NO_FCP. The page did not paint any content. Please ensure you keep the browser window in the foreground during the load and try again. (NO_FCP) PSI

@aem-code-sync
Copy link

aem-code-sync bot commented Jul 21, 2023

Page Scores Audits Google
/thought-leadership PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@aem-code-sync
Copy link

aem-code-sync bot commented Jul 24, 2023

Page Scores Audits Google
/thought-leadership PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

Copy link

@sharathmrft sharathmrft left a comment

Choose a reason for hiding this comment

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

@sahmad-merative @proeung
Hero banner along with the correct image is not appearing
in 'Health Insights' and 'Marketscan' pages-Refer the screenshot below.
quparam

@sahmad-merative sahmad-merative changed the title 753 query param sidebar (DO NOT MERGE BEFORE #274) 753 query param sidebar Jul 24, 2023
@aem-code-sync
Copy link

aem-code-sync bot commented Jul 24, 2023

Page Scores Audits Google
/thought-leadership PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@proeung proeung changed the title 753 query param sidebar feat(thought-leadership): add URL/query parameter to sidebar filters Jul 24, 2023
Copy link

@proeung proeung left a comment

Choose a reason for hiding this comment

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

@sahmad-merative Looks good overall. I just have a couple of comments/suggestions.

blocks/blog-home/blog-home.js Outdated Show resolved Hide resolved
blocks/blog-home/blog-home.js Outdated Show resolved Hide resolved
blocks/thought-leadership-home/thought-leadership-home.js Outdated Show resolved Hide resolved
@aem-code-sync
Copy link

aem-code-sync bot commented Jul 25, 2023

Page Scores Audits Google
/thought-leadership PERFORMANCE A11Y SEO BEST PRACTICES SI FCP LCP TBT CLS PSI

@sharathmrft
Copy link

@sahmad-merative @proeung

Tested all the filters URL Individually its working fine except Hero banner and the Image needs to be displayed as I mentioned above for 'Health Insights' and 'Market scan'

@Shalini-SB @keith-kaplan

Copy link

@proeung proeung left a comment

Choose a reason for hiding this comment

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

@sahmad-merative Code changes look good. Thanks for addressing my feedback!

@sharathmrft The issue you were seeing was related to the page preview caches, where the script for the hero block wasn't loading and returned a 404 (see attached).
Screenshot 2023-07-25 at 3 01 50 PM

I just cleared the caches and the hero banner should be displaying correctly. Please test again - https://753-query-param-sidebar--merative2--sahmad-merative.hlx.page/thought-leadership

@sharathmrft
Copy link

@proeung Looks fine now.

@proeung proeung merged commit 9ecb0fc into hlxsites:feat/thought-leadership Jul 27, 2023
2 checks passed
proeung added a commit that referenced this pull request Jul 31, 2023
* feat(podcast): Add podcast iframe block (#249)

* Podcast block creation.

* Podcast iframe block creation.

* Delay js for podcast.

* Refactoring of code part - 1.

* Refactoring of code part-2.

* CSS regactoring changes.

* Update scripts/delayed.js

Co-authored-by: Putra Bonaccorsi <putra.roeung@gmail.com>

* Spotify and apple podcast csp changes.

* Apple podcast iframe changes.

---------

Co-authored-by: Putra Bonaccorsi <putra.roeung@gmail.com>

* feat(podcast): Add podcast iframe block (#249)

* Podcast block creation.

* Podcast iframe block creation.

* Delay js for podcast.

* Refactoring of code part - 1.

* Refactoring of code part-2.

* CSS regactoring changes.

* Update scripts/delayed.js

Co-authored-by: Putra Bonaccorsi <putra.roeung@gmail.com>

* Spotify and apple podcast csp changes.

* Apple podcast iframe changes.

---------

Co-authored-by: Putra Bonaccorsi <putra.roeung@gmail.com>

* feat(related-documents): add purple theme and adjust rendering logic when there's no post (#250)

* added card fetching functionality to Linkable cards

* fixed lint issue

* lint issues fixes

* lint issues fixed

* Added auto fetching to related documents

* lint issues fixed

* Update linkable-cards.css

* removed commented code

---------

Co-authored-by: Putra Bonaccorsi <putra.roeung@gmail.com>

* feat(thought-leadership): Add "Hero banner" block (#273)

* Podcast block creation.

* Podcast iframe block creation.

* Delay js for podcast.

* Refactoring of code part - 1.

* Refactoring of code part-2.

* CSS regactoring changes.

* Update scripts/delayed.js

Co-authored-by: Putra Bonaccorsi <putra.roeung@gmail.com>

* feat(thought-leadership): Add "Hero banner"

* feat(thought-leadership): Js lint fix for newline.

* feat(hero-banner): Css refactoring and lint fixes.

* feat(hero-banner): JS refactoring.

* feat(hero-banner): Code refactoring changes.

---------

Co-authored-by: Putra Bonaccorsi <putra.roeung@gmail.com>

* feat(thought-leadership): add sidebar filtering and results (#286)

* thought leadership home added

* Added content type

* Fixed lint issues

* Review comments addressed

* Lint issues fixed

* lint issue fixed

* Update blog-home.js

* sorting of filters added

* Revert "sorting of filters added"

This reverts commit eac35e9.

* added sorting to side filters

* removed console from script.js

* Filters selection fixed

* removed sorting and fixed solution categories

* fixed logic of what should be shown in TL

* feat(thought-leadership): change sidebar filter to assettype

* feat(thought-leadership): improve the rendering of sidebar title

* feat(thought-leadership): enhance getAllBlogs and getThoughtLeadership

* feat(thought-leadership): adjust hero block

* feat(thought-leadership): rename card classes

* feat(thought-leadership): use display-title if it exists

* feat(thought-leadership): adjust filter width

* feat(thought-leadership): address QA feedback

* feat(thought-leadership): fix filter reset count

---------

Co-authored-by: Saad Ahmad <sahmed3@merative.com>
Co-authored-by: Putra Bonaccorsi <putra.roeung@gmail.com>

* fix(thought-leadership): result results for content type filter (#294)

* feat(thought-leadership): add URL/query parameter to sidebar filters (#279)

* Added query param deep linking to thought leadership

* lint issues fixed

* feat(thought-leadership): updated latest code

* feat(thought-leadership): fixed filter issue

* review comments fixed

---------

Co-authored-by: Putra Bonaccorsi <putra.roeung@gmail.com>

* fix(thought-leadership): render display-title if it exists (#303)

* fix(thought-leadership): render display-title if it exists

* fix(thought-leadership): fix lint

---------

Co-authored-by: nimithshetty17 <111452145+nimithshetty17@users.noreply.github.com>
Co-authored-by: sahmad-merative <135624814+sahmad-merative@users.noreply.github.com>
Co-authored-by: Amol Anand <amol-anand@users.noreply.github.com>
Co-authored-by: Saad Ahmad <sahmed3@merative.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Thought Leadership Used for work related to Thought Leadership
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants