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.

Sign up for GitHub

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

💄 [#1169] Make images/icons have static sizes in cards #526

Merged
merged 1 commit into from
Mar 23, 2023
Merged

💄 [#1169] Make images/icons have static sizes in cards #526

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/open_inwoner/accounts/templates/accounts/registration_necessary.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

{% block content %}

<div class="registration-grid">
{% render_grid %}
{% render_column span=9 %}
{% render_card tinted=True %}
Expand All @@ -11,5 +12,6 @@ <h1 class="h1">{% trans "Registratie voltooien" %}</h1><br>
{% endrender_card %}
{% endrender_column %}
{% endrender_grid %}
</div>

{% endblock content %}
2 changes: 2 additions & 0 deletions src/open_inwoner/accounts/templates/django_registration/registration_form.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

{% block content %}

<div class="registration-grid">
{% render_grid %}
{% render_column start=5 span=5 %}
{% render_card direction='horizontal' tinted=True %}
Expand All @@ -23,5 +24,6 @@ <h1 class="h3">{% trans "Registreren met E-mail" %}</h1><br>
{% endif %}

{% endrender_grid %}
</div>

{% endblock content %}
4 changes: 3 additions & 1 deletion src/open_inwoner/accounts/templates/registration/login.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@


{% block content %}
<div class="login-grid">
{% render_grid %}
{% render_column start=5 span=5 %}
{% render_card %}
Expand All @@ -24,7 +25,7 @@ <h1 class="h1">{% trans 'Welkom' %}</h1>
{% link href='digid:login' text=_('Inloggen met DigiD') secondary=True icon='arrow_forward' extra_classes="link--digid" %}
{% endrender_card %}
{% endif %}

{% get_solo 'mozilla_django_oidc_db.OpenIDConnectConfig' as oidc_config %}
{% get_solo 'configurations.SiteConfiguration' as site_config %}
{% if oidc_config.enabled %}
Expand Down Expand Up @@ -54,4 +55,5 @@ <h1 class="h1">{% trans 'Welkom' %}</h1>
{% endrender_card %}
{% endrender_column %}
{% endrender_grid %}
</div>
{% endblock content %}
4 changes: 2 additions & 2 deletions src/open_inwoner/pdc/views.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ def page_title(self):
def get_context_data(self, **kwargs):
config = SiteConfiguration.get_solo()

limit = 3 if self.request.user.is_authenticated else 4
limit = 4
Copy link
Contributor

Choose a reason for hiding this comment

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

We can remove this variable now as the limit is not dynamic any more. Let's use the number directly in each queryset.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In future I may have to make a difference between the mobile limit and the desktop limit, which hopefully can be done in the templates somewhere (not sure yet), so I guess this can be cleaned up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vaszig I now see the 'limit' is used 5 times in this file; is it really a good idea to remove the 'limit' variable? I would think it is cleaner to keep it and reuse it instead of the value? (in javascript I would keep the variable, not sure if this is also Python convention).

Copy link
Contributor

Choose a reason for hiding this comment

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

I would use a variable if I wanted first to evaluate something and then reuse it (Not in Python, in every language). Here we don;t need to evaluate something, it's just an integer. But this is my knowledge, @Bartvaderkin can you tell us your opinion please?

Copy link
Contributor

Choose a reason for hiding this comment

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

@vaszig @jiromaykin I would keep the variable, so we can see all the usages are linked to the same value. If we'd put the plain number everywhere you get a magic number situation where it isn't clear which numbers are related or just happen to have the same value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@Bartvaderkin @vaszig Hoorrah! in Javascript I would use a 'const' for numbers like these - so I declare this PR as finished and ready for review again :-)

kwargs.update(categories=Category.objects.published().order_by("name")[:limit])
kwargs.update(product_locations=ProductLocation.objects.all()[:1000])
kwargs.update(
Expand All @@ -86,7 +86,7 @@ def get_context_data(self, **kwargs):
and self.request.user.selected_themes.exists()
):
kwargs.update(
categories=self.request.user.selected_themes.order_by("name")[:3]
categories=self.request.user.selected_themes.order_by("name")[:limit]
)
elif highlighted_categories:
kwargs.update(categories=highlighted_categories)
Expand Down
9 changes: 9 additions & 0 deletions src/open_inwoner/scss/components/CardContainer/CardContainer.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,12 @@
.card-container + h2 {
margin-top: var(--gutter-width);
}

/// Exceptions for forms inside cards

.registration-grid,
.login-grid {
.card {
max-width: 100%;
}
}
9 changes: 6 additions & 3 deletions src/open_inwoner/scss/components/Header/AnchorMenu.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
width: 100%;
list-style: none;
margin: 0;
padding: 0;
padding: var(--spacing-extra-large);
z-index: 1002;

&--desktop {
Expand Down Expand Up @@ -62,11 +62,13 @@

.link {
box-sizing: border-box;
padding: var(--spacing-large);
padding: var(--spacing-medium) var(--spacing-large) var(--spacing-medium)
var(--spacing-large);

@media (min-width: 768px) {
border-left: var(--border-width) solid;
border-color: var(--color-gray-light);
padding: var(--spacing-large);
}
}

Expand All @@ -80,7 +82,8 @@

&--mobile__title {
box-sizing: border-box;
padding: var(--spacing-large);
padding: var(--spacing-large) var(--spacing-large) var(--spacing-medium)
var(--spacing-large);

&.h4 {
color: var(--color-primary);
Expand Down
24 changes: 24 additions & 0 deletions src/open_inwoner/scss/views/Categories.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
.categories {
&__content {
.card-container {
grid-template-columns: repeat(auto-fit, 228px);

.card {
max-width: 360px;
}
}
}

&__products {
margin-top: var(--gutter-width);

.card-container {
margin-top: var(--gutter-width);
grid-template-columns: repeat(var(--card-columns), 1fr);

.card {
max-width: 100%;
}
}
}
}
11 changes: 11 additions & 0 deletions src/open_inwoner/scss/views/Home.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
/// Cards on Home Page and Theme page

.home {
.card-container {
grid-template-columns: repeat(auto-fit, 228px);
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we could add a specific rule for the mobile version as it seems a bit weird to my eyes, but you know better.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@vaszig For mobile: horizontal scrolling will be added later in a different issue so it will look better, but the cards will need to remain this same size.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think @alextreme ?

Copy link
Member

Choose a reason for hiding this comment

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

This is fine for now, the scrolling is something Jiro will continue on but these changes are a necessary first step for that.


.card {
max-width: 360px;
}
}
}
2 changes: 2 additions & 0 deletions src/open_inwoner/scss/views/_index.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
@import './App.scss';
@import './body';
@import './Categories.scss';
@import './Home.scss';
@import './Plans.scss';
@import './product_detail';
@import './view';
64 changes: 34 additions & 30 deletions src/open_inwoner/templates/pages/category/detail.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,39 +10,43 @@
{% endblock header_image %}

{% block content %}
<h1 class="h1">
{{ object.name }}
{% if request.user.is_staff %}
{% button icon="edit" text=_("Open in admin") hide_text=True href="admin:pdc_category_change" object_id=object.pk %}
{% endif %}
</h1>
<p class="p">{{ object.description|linebreaksbr }}</p>
<div class="categories__content">
<h1 class="h1">
{{ object.name }}
{% if request.user.is_staff %}
{% button icon="edit" text=_("Open in admin") hide_text=True href="admin:pdc_category_change" object_id=object.pk %}
{% endif %}
</h1>
<p class="p">{{ object.description|linebreaksbr }}</p>

{% if subcategories %}
{% card_container subcategories=subcategories parent_category=object %}
{% endif %}
{% if subcategories %}
{% card_container subcategories=subcategories parent_category=object %}
{% endif %}

{% if products %}
{% card_container products=products small=True parent=object %}
{% endif %}
{% if products %}
<div class="categories__products">
{% card_container products=products small=True parent=object %}
</div>
{% endif %}

{% if category.question_set.all %}
{% render_grid %}
{% render_column span=6 %}
{% faq category.question_set.all %}
{% endrender_column %}
{% endrender_grid %}
{% endif %}
{% if category.question_set.all %}
{% render_grid %}
{% render_column span=6 %}
{% faq category.question_set.all %}
{% endrender_column %}
{% endrender_grid %}
{% endif %}

{% if questionnaire_roots %}
<div class="grid">
<div class="column column--start-1 column--span-6 ">
<aside class="questionnaire">
<h2 class="h2">{{configurable_text.questionnaire_page.select_questionnaire_title}}</h2>
{% optional_paragraph configurable_text.questionnaire_page.select_questionnaire_intro %}
{% questionnaire root_nodes=questionnaire_roots %}
</aside>
{% if questionnaire_roots %}
<div class="grid">
<div class="column column--start-1 column--span-6 ">
<aside class="questionnaire">
<h2 class="h2">{{configurable_text.questionnaire_page.select_questionnaire_title}}</h2>
{% optional_paragraph configurable_text.questionnaire_page.select_questionnaire_intro %}
{% questionnaire root_nodes=questionnaire_roots %}
</aside>
</div>
</div>
</div>
{% endif %}
{% endif %}
</div>
{% endblock content %}
8 changes: 5 additions & 3 deletions src/open_inwoner/templates/pages/category/list.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,10 @@
{% load card_tags %}

{% block content %}
<h1 class="h1">{{configurable_text.theme_page.theme_title}}</h1>
<p class="p">{{configurable_text.theme_page.theme_intro|linebreaksbr}}</p>
<div class="categories__content">
<h1 class="h1">{{configurable_text.theme_page.theme_title}}</h1>
<p class="p">{{configurable_text.theme_page.theme_intro|linebreaksbr}}</p>

{% card_container categories=object_list %}
{% card_container categories=object_list %}
</div>
{% endblock content %}
4 changes: 3 additions & 1 deletion src/open_inwoner/templates/pages/home.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
{% endblock header_image %}

{% block content %}
<div class="home">
{% block user_content %}
<div class="grid__welcome">
<h1 class="h1">{{configurable_text.home_page.home_welcome_title}}</h1>
Expand All @@ -24,7 +25,7 @@ <h2 class="h2">
<p class="p">{{configurable_text.home_page.home_theme_intro|linebreaksbr}}</p>

{% if request.user.is_authenticated %}
{% card_container categories=categories columns=3 image_object_fit="cover" %}
{% card_container categories=categories columns=4 image_object_fit="cover" %}
{% else %}
{% card_container categories=categories image_object_fit="cover" %}
{% endif %}
Expand All @@ -45,4 +46,5 @@ <h2 class="h2">{{configurable_text.home_page.home_map_title}}</h2>
{% with centroid=product_locations.get_centroid %}
{% map centroid.lat centroid.lng geojson_feature_collection=product_locations.get_geojson_feature_collection %}
{% endwith %}
</div>
{% endblock %}
2 changes: 1 addition & 1 deletion src/open_inwoner/templates/pages/user-home.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ <h2 class="h2">
</h2>

{% if plans %}
<div class="plans-cards card-container card-container--columns-3">
<div class="plans-cards card-container card-container--columns-4">
{% for plan in plans %}
{% render_card image_object_fit="cover" %}
<h3 class="h3">{{ plan.title }}</h3>
Expand Down