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

🗑️ [#2062] Remove card templatetags #1010

Merged
merged 2 commits into from
Feb 12, 2024

Conversation

stevenbal
Copy link
Contributor

@stevenbal stevenbal commented Feb 8, 2024

@stevenbal stevenbal marked this pull request as draft February 8, 2024 08:58
@codecov-commenter
Copy link

codecov-commenter commented Feb 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (fefb1f5) 94.74% compared to head (a7ebcbf) 94.74%.

❗ Current head a7ebcbf differs from pull request most recent head 7a7a817. Consider uploading reports for the commit 7a7a817 to get more accurate results

Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1010      +/-   ##
===========================================
- Coverage    94.74%   94.74%   -0.01%     
===========================================
  Files          872      872              
  Lines        30571    30544      -27     
===========================================
- Hits         28965    28939      -26     
+ Misses        1606     1605       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@stevenbal stevenbal force-pushed the cleanup/2062-remove-card-templatetags branch 3 times, most recently from 6af9edd to a7ebcbf Compare February 8, 2024 10:18
@stevenbal stevenbal force-pushed the cleanup/2062-remove-card-templatetags branch from a7ebcbf to 57314b1 Compare February 8, 2024 10:37
@stevenbal stevenbal force-pushed the cleanup/2062-remove-card-templatetags branch from 7a7a817 to 997c18c Compare February 8, 2024 10:53
@stevenbal stevenbal marked this pull request as ready for review February 8, 2024 10:54
Copy link
Contributor

@jiromaykin jiromaykin left a comment

Choose a reason for hiding this comment

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

Approved for the clean-up + and the location card currently contains clickable phone/mail links, which makes it a bit different from the other cards, so that's good to have as a separate construct.

@@ -55,113 +26,3 @@ def render_card(parser, token):
nodelist = parser.parse(("endrender_card",))
parser.delete_first_token()
return ContentsNode(nodelist, "components/Card/RenderCard.html", **context_kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

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

Very pleasant clean-up :-)

@@ -0,0 +1,16 @@
{% load link_tags string_tags %}
<div class="card card__body">
{% if location_name %}
Copy link
Contributor

Choose a reason for hiding this comment

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

I see this card is in the 'old' style and contains a link inside, (as opposed to all the other cards that are entirely clickable with a surrounding anchor instead) but then again: we'll indeed need to keep the phone/mail as clickable items.

@alextreme alextreme merged commit 79332e4 into develop Feb 12, 2024
14 checks passed
@alextreme alextreme deleted the cleanup/2062-remove-card-templatetags branch February 12, 2024 09:25
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.

5 participants