Adds carousel text block #11541
Adds carousel text block #11541
Conversation
kevinhowbrook
force-pushed
the
feature/8001-listing-block-adjustments
branch
2 times, most recently
from
d30d371 to
2a749a6
Compare
kevinhowbrook
force-pushed
the
feature/9030-text-and-carousel-block
branch
from
d667786 to
7f2afc9
Compare
| </div> | ||
| </div> | ||
| </div> | ||
| {% endblock block_content %} |
There was a problem hiding this comment.
Again - chris will be doing the FE for this
kevinhowbrook
force-pushed
the
feature/9030-text-and-carousel-block
branch
2 times, most recently
from
29e72b8 to
9ebe084
Compare
There was a problem hiding this comment.
Looks good @kevinhowbrook. Just minor questions and suggestions.
| # Use specific link fields for the CTA on the block as opposed to the | ||
| # common.link_blocks.LabelledExternalLinkBlock so it can be marked as | ||
| # required=False. | ||
| link_url = blocks.URLBlock(help_text="A CTA URL for a link displayed", required=False) |
There was a problem hiding this comment.
There is no need to internal links / page chooser?
There was a problem hiding this comment.
That's not a requirement at the moment, but we'll see with QA
| @@ -53,6 +53,7 @@ class MozfestPrimaryPage(FoundationMetadataPageMixin, FoundationBannerInheritanc | |||
| ("tabbed_profile_directory", customblocks.TabbedProfileDirectory()), | |||
| ("newsletter_signup", customblocks.NewsletterSignupBlock()), | |||
| ("stats_block", mozfest_blocks.StatisticsBlock()), | |||
| ("carousel_text_block", mozfest_blocks.CarouselTextBlock()), | |||
There was a problem hiding this comment.
Looking at the existing block names above, what do you think about not including _block in the bock name?
There was a problem hiding this comment.
Yea that's a good point, honestly couldn't decide but that makes sense, even with the rebase drama it will cause it's worth doing 👍
There was a problem hiding this comment.
Sorry, did not know rebase drama would follow 😅
| heading = blocks.CharBlock(help_text="Heading for the block.", required=False) | ||
|
|
||
| text = blocks.RichTextBlock(features=["bold", "italic", "link"]) | ||
| carousel_images = blocks.ListBlock(customblocks.ImageBlock(), max_num=4) |
There was a problem hiding this comment.
Nitpick What do you think about grouping the definitions here similar to how they will show? In this case moving the carousel_images below all the other fields.
There was a problem hiding this comment.
👍 good spot, thanks
kevinhowbrook
force-pushed
the
feature/9030-text-and-carousel-block
branch
from
a6ccca5 to
49a1ec3
Compare
Description
This PR adds a new block type, "Carousel and Text block" for use on the mozfest site.
Link to sample test page: http://mozfest.localhost:8000/en/
Related PRs/issues: https://torchbox.monday.com/boards/1334760528/pulses/1336799030
Checklist
Tests
Changes in Models:
Documentation:
Merge Method
💡❗Remember to use squash merge when merging non-feature branches into
main