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

Bump Wagtail to 6.4 #40

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
13 changes: 6 additions & 7 deletions fixtures/demo.json
Original file line number Diff line number Diff line change
Expand Up @@ -1080,6 +1080,7 @@
"collection": 1,
"title": "Placeholder Image",
"file": "original_images/wagtail.webp",
"description": "A Pied Wagtail walking with its left foot in the air. Copyright The Hall of EINAR.",
Copy link
Member Author

Choose a reason for hiding this comment

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

I found out that the image is from https://www.thehallofeinar.com/2022/03/pied-wagtail/, and there's a copyright watermark. We should probably replace the image with something in public domain, e.g. from Wikimedia Commons.

Copy link
Contributor

Choose a reason for hiding this comment

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

I've used this one before because it was taken in the UK. The license requires attribution and a link to the source. So we'll have to add those if we decide to use this one: https://commons.wikimedia.org/wiki/File:Wagtail_-_geograph.org.uk_-_6524549.jpg

Copy link
Member Author

Choose a reason for hiding this comment

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

We can find ones that don't require attribution, such as this one: https://commons.wikimedia.org/wiki/File:White_wagtail_(Motacilla_alba).jpg. Using an image with the least restrictive license is probably more important than where the photo was taken.

"width": 2048,
"height": 1365,
"created_at": "2024-02-26T15:28:56.871Z",
Expand All @@ -1089,8 +1090,7 @@
"focal_point_width": null,
"focal_point_height": null,
"file_size": null,
"file_hash": "",
"alternative_text": ""
"file_hash": ""
}
},
{
Expand All @@ -1100,6 +1100,7 @@
"collection": 1,
"title": "frank",
"file": "original_images/frank.png",
Copy link
Member Author

Choose a reason for hiding this comment

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

I don't know where the image is from 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Me either. We might want to replace it with an image from the bakery demo to be safe.

"description": "Portrait image of Frank Nigelson",
"width": 430,
"height": 436,
"created_at": "2024-02-26T15:30:40.198Z",
Expand All @@ -1111,8 +1112,7 @@
"focal_point_width": null,
"focal_point_height": null,
"file_size": 325553,
"file_hash": "54714057fdff3731850126bbcbd93d66d5b3a59a",
"alternative_text": "Portrait image of Frank Nigelson"
"file_hash": "54714057fdff3731850126bbcbd93d66d5b3a59a"
}
},
{
Expand All @@ -1122,6 +1122,7 @@
"collection": 1,
"title": "mackerel",
"file": "original_images/mackerel.webp",
"description": "A school of Pacific Mackerel swimming in dark waters.",
Copy link
Member Author

Choose a reason for hiding this comment

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

This one seems to be from https://cma.recreation.parks.lacity.gov/marine-life/southern-california-species/fish/pacific-mackerel. Again we should probably replace this.

Copy link
Contributor

Choose a reason for hiding this comment

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

You are correct that we should replace this. State and local governments can hold copyrights in the United States. The federal government, however, cannot and all their images are in the public domain unless a copyright by the photographer is cited. NOAA's fish pictures were all too small though.

This jack mackerel image could be a good substitute. Again we would have to provide attribution. https://commons.wikimedia.org/wiki/File:Pacific_Jack_Mackerel_School,_2007.jpg

Copy link
Contributor

Choose a reason for hiding this comment

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

I discovered today that there actually are NOT more mackerel in Kent than people in China. Not the biggest priority but something to be aware of: https://media.mcsuk.org/documents/Mackerel_no_longer_sustainable_choice_says_environmental_charity.pdf

Copy link
Member Author

Choose a reason for hiding this comment

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

"width": 640,
"height": 457,
"created_at": "2024-02-26T15:54:21.403Z",
Expand All @@ -1133,8 +1134,7 @@
"focal_point_width": 182,
"focal_point_height": 119,
"file_size": 22162,
"file_hash": "c9e7cea533cf90f49c3ef8f71c38dec4f43aa168",
"alternative_text": ""
"file_hash": "c9e7cea533cf90f49c3ef8f71c38dec4f43aa168"
}
},
{
Expand Down Expand Up @@ -14058,4 +14058,3 @@
}
}
]

4 changes: 2 additions & 2 deletions project_name/images/migrations/0001_initial.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ class Migration(migrations.Migration):
('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')),
('title', models.CharField(max_length=255, verbose_name='title')),
('file', wagtail.images.models.WagtailImageField(height_field='height', upload_to=wagtail.images.models.get_upload_to, verbose_name='file', width_field='width')),
('description', models.CharField(blank=True, default='', max_length=255, verbose_name='description')),
('width', models.IntegerField(editable=False, verbose_name='width')),
('height', models.IntegerField(editable=False, verbose_name='height')),
('created_at', models.DateTimeField(auto_now_add=True, db_index=True, verbose_name='created at')),
Expand All @@ -35,8 +36,7 @@ class Migration(migrations.Migration):
('focal_point_height', models.PositiveIntegerField(blank=True, null=True)),
('file_size', models.PositiveIntegerField(editable=False, null=True)),
('file_hash', models.CharField(blank=True, db_index=True, editable=False, max_length=40)),
('alternative_text', models.CharField(blank=True, help_text="Provide a text alternative for this image for visually impaired users.<br />For advice and best practice, visit <a href='https://moz.com/learn/seo/alt-text' target='_blank' rel='noreferrer noopener'>https://moz.com/learn/seo/alt-text</a>", max_length=200)),
('collection', models.ForeignKey(default=wagtail.models.collections.get_root_collection_id, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='wagtailcore.collection', verbose_name='collection')),
('collection', models.ForeignKey(default=wagtail.models.get_root_collection_id, on_delete=django.db.models.deletion.CASCADE, related_name='+', to='wagtailcore.collection', verbose_name='collection')),
Copy link
Member Author

Choose a reason for hiding this comment

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

What's changed:

('tags', taggit.managers.TaggableManager(blank=True, help_text=None, through='taggit.TaggedItem', to='taggit.Tag', verbose_name='tags')),
('uploaded_by_user', models.ForeignKey(blank=True, editable=False, null=True, on_delete=django.db.models.deletion.SET_NULL, to=settings.AUTH_USER_MODEL, verbose_name='uploaded by user')),
],
Expand Down
20 changes: 2 additions & 18 deletions project_name/images/models.py
Original file line number Diff line number Diff line change
@@ -1,31 +1,15 @@
from PIL import ImageOps
from django.db import models
from django.utils.safestring import mark_safe
from wagtail import hooks
from wagtail.search import index
from wagtail.images.models import AbstractImage, AbstractRendition, Image
from wagtail.images.image_operations import FilterOperation


class CustomImage(AbstractImage):
alternative_text = models.CharField(
blank=True,
max_length=200,
help_text=mark_safe(
"Provide a text alternative for this image for visually "
"impaired users."
"<br />For advice and best practice, visit "
"<a href='https://moz.com/learn/seo/alt-text' target='_blank' "
"rel='noreferrer noopener'>"
"https://moz.com/learn/seo/alt-text</a>"
),
)

admin_form_fields = Image.admin_form_fields + (
"alternative_text",
)
admin_form_fields = Image.admin_form_fields

search_fields = AbstractImage.search_fields + [index.SearchField("alternative_text")]
search_fields = AbstractImage.search_fields + [index.SearchField("description")]
Copy link
Member Author

Choose a reason for hiding this comment

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

AbstractImage doesn't include description in the search_fields for some reason (was that intentional?)



class Rendition(AbstractRendition):
Expand Down
4 changes: 2 additions & 2 deletions project_name/news/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@

from wagtail.fields import StreamField
from {{ project_name }}.utils.models import BasePage, ArticleTopic
from {{ project_name }}.utils.blocks import ImageBlock, StoryBlock, FeaturedArticleBlock
from {{ project_name }}.utils.blocks import CaptionedImageBlock, StoryBlock, FeaturedArticleBlock


class ArticlePage(BasePage):
Expand Down Expand Up @@ -37,7 +37,7 @@ class ArticlePage(BasePage):
)
introduction = models.TextField(blank=True)
image = StreamField(
[("image", ImageBlock())],
[("image", CaptionedImageBlock())],
blank=True,
max_num=1,
)
Expand Down
4 changes: 4 additions & 0 deletions project_name/settings/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -252,6 +252,10 @@ def get_first_env(*keys, default=None):
# Limit how large a file can be spooled into memory before it's written to disk.
AWS_S3_MAX_MEMORY_SIZE = 2 * 1024 * 1024 # 2MB

# Django sets a maximum of 1000 fields per form by default, but particularly complex page models
# can exceed this limit within Wagtail's page editor.
DATA_UPLOAD_MAX_NUMBER_FIELDS = 10_000


# Wagtail settings

Expand Down
10 changes: 5 additions & 5 deletions project_name/utils/blocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from wagtail import blocks
from wagtail.blocks.struct_block import StructBlockValidationError
from wagtail.documents.blocks import DocumentChooserBlock
from wagtail.images.blocks import ImageChooserBlock
from wagtail.images.blocks import ImageBlock, ImageChooserBlock
from wagtail.snippets.blocks import SnippetChooserBlock

from {{ project_name }}.utils.struct_values import CardStructValue, LinkStructValue
Expand All @@ -29,7 +29,7 @@ class Meta:
template = "components/accordion/accordion.html"


class ImageBlock(blocks.StructBlock):
class CaptionedImageBlock(blocks.StructBlock):
image = ImageChooserBlock()
image_alt_text = blocks.CharBlock(
required=False,
Expand Down Expand Up @@ -125,7 +125,7 @@ class Meta:

class FeaturedArticleBlock(blocks.StructBlock):
Copy link
Member Author

Choose a reason for hiding this comment

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

This block isn't used anywhere, it's imported in news/models.py but not used. I'm not sure how the project template is supposed to be used. Do we only provide the block classes, and users get to choose which ones to include in the StreamField definition? Most of the blocks don't have meaningful templates too (#38).

Copy link
Contributor

Choose a reason for hiding this comment

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

I want to create a template for this block because I feel like it's a valuable one to show people how to use and how to incorporate into their projects.

When I went to shoot a tutorial on the starter kit, I was thinking I would have to add the extra blocks myself and surprised to find a bunch of extra blocks without templates or malfunctioning templates were included.

I think this one is worth fixing up. I've gone ahead and provided fixes for several others listed in #38 as well.

link = ArticlePageLinkBlock()
image = ImageChooserBlock(
image = ImageBlock(
required=False,
help_text="Set to override the image of the chosen article page.",
)
Expand All @@ -151,7 +151,7 @@ class Meta:

class BaseSectionBlock(blocks.StructBlock):
heading = blocks.CharBlock(
form_classname="title",
form_classname="title",
icon="title",
required=True
) # Should use H2s only
Expand Down Expand Up @@ -182,7 +182,7 @@ class Meta:

class CTASectionBlock(blocks.StructBlock):
heading = blocks.CharBlock(
form_classname="title",
form_classname="title",
icon="title",
required=True
)
Expand Down
2 changes: 1 addition & 1 deletion requirements.txt
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
django>=4.2,<5.2
wagtail>=6.2,<6.3
wagtail>=6.4,<6.5
dj-database-url
psycopg[binary]
whitenoise
Expand Down
6 changes: 3 additions & 3 deletions templates/pages/article_page.html
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ <h1 class="font-serif4 font-bold text-4xl lg:text-7xl">
height="{{ author_avatar.height }}"
alt="{{ author_avatar.alt }}"
class="w-10 h-10 rounded-full"
/>
/>
{{ page.author.title }}
</div>
</div>
Expand All @@ -58,10 +58,10 @@ <h1 class="font-serif4 font-bold text-4xl lg:text-7xl">
/>
</picture>



</div>
{% if value.caption %}
{% if page.image.0.value.caption %}
<p class="text-sm leading-6 py-5">{{ page.image.0.value.caption }}</p>
Comment on lines +64 to 65
Copy link
Member Author

Choose a reason for hiding this comment

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

Note: I had an earlier commit that replaces this with figure and figcaption, but I removed it because we might want to do it in #41 instead. I don't know why we write the template for the image block directly on the page template instead of the image block's template.

Maybe because the image block is meant to be more general-purpose rather than a big main image? Or maybe because the code originally used a foreign key and was replaced with a StreamField that only contains the ImageBlock 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

But what if people want to add an ImageBlock to their StreamField? Would we encourage them to add images in their article body with RichTextBlock instead? We have to think about how editors are adding images to their content further down the line and I feel like it's slightly more intuitive to select an image block from the StreamField menu than to add it via another method.

Copy link
Contributor

Choose a reason for hiding this comment

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

it just occurred to me that the default ImageBlock could be called into StreamField. I think the argument for having a separate one though would be more precise control of the styling and how the image is displayed.

{% endif %}
</div>
Expand Down