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

Update Spotlight homepage posts to Ideas, including cta link text #12663

Merged
merged 19 commits into from
Sep 10, 2024

Conversation

ramram-mf
Copy link
Collaborator

@ramram-mf ramram-mf commented Jul 26, 2024

Description

This PR updates the homepage Spotlight posts, renaming the block to Ideas and adding a CTA Link Text field, which is displayed on the lower part of the post component.

Current state:
image
image

Updated state:
image
image

# To test

  1. Log in to the preview app CMS with admin2:admin2 credentials.
  2. Browse the homepage edit page. CTA Link Text be the last field input inside the Ideas card. Publish.
  3. Homepage should be updated with the Ideas header and CTA latest values.

Link to sample test page: https://foundation-s-tp1-981-ad-tfr9ki.herokuapp.com/
Related PRs/issues: #12601

┆Issue is synchronized with this Jira Story

@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-1rqlfa July 26, 2024 20:33 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-1rqlfa July 26, 2024 20:51 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-1rqlfa July 27, 2024 00:02 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-1rqlfa July 29, 2024 16:34 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-1rqlfa July 29, 2024 16:44 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-1rqlfa July 29, 2024 17:06 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-jncdvw July 29, 2024 17:28 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-jncdvw July 29, 2024 17:49 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-jncdvw July 29, 2024 18:01 Inactive
@ramram-mf ramram-mf self-assigned this Jul 29, 2024
@ramram-mf ramram-mf marked this pull request as ready for review July 29, 2024 18:33
Copy link
Collaborator

@robdivincenzo robdivincenzo left a comment

Choose a reason for hiding this comment

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

Great work on this @ramram-mf! Code looks good, and solves the issue. But I had a couple of thoughts regarding the long-term of this element.

First, I noticed we still have mixed use of Spotlight image and Ideas image, etc. (see screenshot below). If we are moving to Ideas I wonder if we should rename it in full (templates & models too) to avoid confusion with this element in the future.

The second thought I had was if we're adding a cta link text, maybe we just should migrate the blog post and cta fields to just use to the new LinkBlock field (the one @danielfmiranda has been migrating a bunch the past couple sprints). For example, we had a request come in last week to link these sections to a PNI article page, but since these fields are strictly limited to a Blog Page, we weren't able to accommodate the request. If it were a LinkBlock, we'd have a lot more flexibility to do that and also include the "Read More" cta text.

Curious to hear your thoughts!

Screenshot 2024-07-30 at 10 32 57 AM

Copy link
Collaborator

@nancyt1 nancyt1 left a comment

Choose a reason for hiding this comment

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

  1. The current text links don't go to the corresponding page for me. It just reloads the homepage.
  2. Are you able to make the height of the image the same height as the right column, however long it might get? It's ok if it crops further
SCR-20240729-mnjk

@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-jncdvw July 30, 2024 17:33 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-tfr9ki September 9, 2024 04:12 Inactive
@ramram-mf
Copy link
Collaborator Author

@robdivincenzo So, I explored changing the blog field to a LinkBlock streamfield, but that made getting and displaying the author data an inconsistent process, which would not align with the design refresh scope in my opinion. Model and field names are updated, reflecting the new Ideas section naming, but the PR is failing the Wagtail CI actions workflow, stating some error with some non existing residual file from my migration olympics. Any thoughts on how to force the CI workflow to pull the latest branch state?

@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-tfr9ki September 9, 2024 18:32 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-tfr9ki September 9, 2024 19:18 Inactive
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-tfr9ki September 9, 2024 20:25 Inactive
…b-mf:MozillaFoundation/foundation.mozilla.org into TP1-981-add-text-links-to-ideas-section-cards
@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-tfr9ki September 9, 2024 20:46 Inactive
@nancyt1
Copy link
Collaborator

nancyt1 commented Sep 9, 2024

@ramram-mf The text links seem to be missing on mobile, is that intentional?

@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-tfr9ki September 9, 2024 23:22 Inactive
Copy link
Collaborator

@robdivincenzo robdivincenzo left a comment

Choose a reason for hiding this comment

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

Ty for cleaning up the naming and looking into the LinkBlock idea, code LGTM! 🚀

@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-tfr9ki September 10, 2024 16:32 Inactive
@ramram-mf
Copy link
Collaborator Author

@ramram-mf The text links seem to be missing on mobile, is that intentional?

@nancyt1 Updated the mobile layout so it now looks like this:
image

@ramram-mf ramram-mf temporarily deployed to foundation-s-tp1-981-ad-tfr9ki September 10, 2024 18:37 Inactive
@ramram-mf ramram-mf merged commit 94daaaa into main Sep 10, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants