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

8813 - MozFest tickets block #11537

Merged
merged 4 commits into from
Dec 14, 2023

Conversation

chris-lawton
Copy link
Collaborator

Description

  • Changes the mozfest_base.html to have a single column layout
  • Adds the page template for a new streamfield block called mozfest_tickets_block which can show up to 3 tiers of tickets, each with a cost, tier name, text, link and optional sticker.
  • BE integration will come a future ticket

Desktop/Tablet/Mobile screens:

Details

Screenshot 2023-12-13 at 10 04 10

Screenshot 2023-12-13 at 10 04 50

Screenshot 2023-12-13 at 10 04 59

Link to sample test page:
Related PRs/issues: #

Checklist

Tests

  • Is the code I'm adding covered by tests?

Changes in Models:

  • Did I update or add new fake data?
  • Did I squash my migration?
  • Are my changes backward-compatible. If not, did I schedule a deploy with the rest of the team?

Documentation:

  • Is my code documented?
  • Did I update the READMEs or wagtail documentation?

Merge Method
💡❗Remember to use squash merge when merging non-feature branches into main

@chris-lawton chris-lawton requested a review from tbrlpld December 13, 2023 10:21
Copy link
Collaborator

@tbrlpld tbrlpld left a comment

Choose a reason for hiding this comment

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

Looks good to me @chris-lawton

<div class="tw-bg-festival-black-100 tw-p-8 tw-h-full tw-flex tw-flex-col tw-items-start">
<h3 class="tw-h2-heading">{{ tier.price }}</h3>
<p class="tw-mt-0">{{ tier.group }}</p>
<div class="[&>*]:tw-text-sm">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Interesting. Does the font size not cascade down? Or it there another rule at place we have to override?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The text that appears here will come from a rich text input which means I can't apply classes directly to any of the elements that get output - p or li tags for example. The default font-size set on these elements is too large compared to the design hence why the font size needs to be set higher up on this element. You have made me realise that the use of the child combinator is wrong due to the rich-text div that will be output.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Updated in e3ac3cf

<div class="tw-row">
{% for tier in self.tickets %}
{% with count=self.tickets|length %}
<div class="tw-mb-12 tw-w-full{% if count > 2 %} large:tw-w-1/3{% else %} large:tw-w-1/2{% endif %} tw-px-8{% if tier.sticker_text %} tw-relative{% endif %}">
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we maybe need space between the class names and the curlies? I think the Tailwind might not find the class names when they touch curlies.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

They look to output ok like this. Here's a quick test I ran:

<h2 class="tw-mb-14{% if test_var %} tw-text-transparent{% endif %}">A title</h2>
output:
<h2 class="tw-mb-14">A title</h2>

<h2 class="tw-mb-14{% if not test_var %} tw-text-transparent{% endif %}">A title</h2>
output:
<h2 class="tw-mb-14 tw-text-transparent">A title</h2>

Copy link
Collaborator

Choose a reason for hiding this comment

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

I think when Tailwind generates the production stylesheet (.css) by parsing our template files it will see the curlies. I don't think the parsing happens against the server response but the actual file content 🤔 so it might include the curlies touching the classnames.

I think in that case, the classname might be missing from the final stylesheet file Tailwind does not know we used the classes. They could still show up if they are used elsewhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Ah ok, that might be why my test was working then. I'll tidy this up in another MR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@chris-lawton chris-lawton merged commit a43cff2 into integration/mozfest-2024 Dec 14, 2023
6 checks passed
@chris-lawton chris-lawton deleted the feature/8813-tickets branch December 14, 2023 10:04
@chris-lawton chris-lawton mentioned this pull request Dec 15, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants