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

Feature: Dynamic web stories from backend #892

Merged
merged 50 commits into from
Dec 28, 2024

Conversation

amovar18
Copy link
Collaborator

@amovar18 amovar18 commented Dec 12, 2024

Description

This PR aims to build dynamic stories from the backend response.

Relevant Technical Choices

  • Fetch the set of stories from the backend parse the response and generate a single story object that looks like the following:
{
  heroImage: string;
  publisherLogo: string;
  publisherName: string;
  storyTitle: string;
  storyUrl: string;
}
  • This PR also fetches the filters of Category, Author and Tags from API calls to the backend.

Testing Instructions

  • Clone this branch.
  • In the terminal run npm i && npm run build:ext.
  • Now go to example.com and open the DevTools and open Privacy Sandbox tab.
  • You should see a section in sidebar titled as Explorable Explanation with Web-Stories Icon.
  • Click on the section. A set of stories with their hero image should be rendered on the main content.
  • Clicking on the hero image will result in the story opening in a light box.

Additional Information:

The following things are to be done:

  • Fetch tags from the backend.
  • Add infinite scroll to the stories.

Screenshot/Screencast


Checklist

  • I have thoroughly tested this code to the best of my abilities.
  • I have reviewed the code myself before requesting a review.
    - [ ] This code is covered by unit tests to verify that it works as intended.
  • The QA of this PR is done by a member of the QA team (to be checked by QA).

Fixes #897

@amovar18 amovar18 mentioned this pull request Dec 13, 2024
4 tasks
@amovar18 amovar18 self-assigned this Dec 16, 2024
@amovar18 amovar18 added this to the v0.13.0 milestone Dec 16, 2024
@amovar18 amovar18 marked this pull request as ready for review December 23, 2024 08:34
@mohdsayed
Copy link
Collaborator

@amovar18

  1. Initially it shows only 4 stories with arrow down indicator however scrolling down doesn't work.

image

It works in vertical mode however the placement of arrow down doesn't look right.

image

  1. Is it a requirement to load only 4 stories in the beginning?
    image

  2. When no stories are found, it should say that instead of showing empty screen
    image

  3. When more stories are loading, it should indicate that using some text or loader.

  4. Is it possible to close the popup on esc button press?

image

@mohdsayed
Copy link
Collaborator

mohdsayed commented Dec 25, 2024

Some more improvements:

  1. When there are no stories, down arrow should not appear.
  2. Where there are no stories in search, arrow up should not appear.
  3. When it's waiting for stories to load, it should show loader.
    image

Recommendations:

  • Arrow down can be clickable and should load more stories.
  • Arrow up should scroll to the top.
  • Increase initial stories count to at least 8 so that it doesn't show empty space and user has something to scroll.

No Stories found text is not styled.
The arrow is still on the extreme right in the vertical mode.
Click on the arrow down is loading more stories, but it would be good if we also scroll to the bottom to show new stories.
After new stories are loaded and there are no more stories left, we should hide the arrow down.
After the we click on arrow up and the user scrolls to the top, arrow up should update as the user can no longer scroll up.
Add loader to display loader for load more.
Copy link
Collaborator

@mohdsayed mohdsayed left a comment

Choose a reason for hiding this comment

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

Needs few improvements.

@mohdsayed mohdsayed merged commit dd1739f into release/v0.13.0 Dec 28, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

3 participants