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

Allow ordering of logos in theme footer #1121

Closed
BeritJanssen opened this issue Jun 14, 2024 · 4 comments · Fixed by #1188
Closed

Allow ordering of logos in theme footer #1121

BeritJanssen opened this issue Jun 14, 2024 · 4 comments · Fixed by #1188
Assignees

Comments

@BeritJanssen
Copy link
Collaborator

Right now, logos are added in the order of their pk in the database. It would be good if there was a more elegant way to control their ordering.

@drikusroor
Copy link
Contributor

drikusroor commented Jun 14, 2024

We can do it the very simple way: Add a sort number to the Image model, but this can have nasty side effects if you are using the image in several places

Another way might be: Footer.logos -> SortedImage (with sort prop) -> Image

A more complex but generic way might be to introduce ImageGalleries:

Footer.logos -> ImageGallery -> ImagegalleryImage (with sort prop) -> Image

This allows us to share the ImageGallery for both Toontje Hoger & Toontje Hoger Kids (if they use the same logos that is)

@BeritJanssen
Copy link
Collaborator Author

I was also thinking of sorting by name, which can be done quite easily. I do think we'll deal with logos being reused, which would mean we would have to add multiple copies of the same image. On the other hand, it's unlikely that whole logo sets will be reused.

So I think the implementation of a OrderedImage or similar would be best. NB I think we should always call ordering fields index, as it's clearest that lower == first.

@drikusroor
Copy link
Contributor

I was also thinking of sorting by name, which can be done quite easily. I do think we'll deal with logos being reused, which would mean we would have to add multiple copies of the same image. On the other hand, it's unlikely that whole logo sets will be reused.

So I think the implementation of a OrderedImage or similar would be best. NB I think we should always call ordering fields index, as it's clearest that lower == first.

I did a bit of research and I did fin a potential problem. If we use a model OrderedImage, it will de facto be the many to many model that already exists now, but with an index. This means that the model will have an FK to FooterConfig, e.g.:

class OrderedImage(models.Model):
    footer_config = models.ForeignKey(FooterConfig, on_delete=models.CASCADE)
    image = models.ForeignKey('image.Image', on_delete=models.CASCADE)
    index = models.PositiveIntegerField()

This will then also mean that we cannot use OrderedImage generically for potential other ordered images on other models. Is that okay or do we want it to be more generic?

If we do want it to be reusable for other models in the future, we can go for the earlier mentioned ImageGallery option, or we can use Django's "GenericForeignKey" in combination with a "ContentType":

class OrderedImage(models.Model):
    content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) # This will be the FK of the FooterConfig model (not the object)
    object_id = models.PositiveIntegerField() # This will be the FK of the FooterConfig object
    content_object = GenericForeignKey('content_type', 'object_id')
    image = models.ForeignKey('image.Image', on_delete=models.CASCADE)
    index = models.PositiveIntegerField()

    class Meta:
        ordering = ['index']
        unique_together = (('content_type', 'object_id', 'image'),)

@BeritJanssen
Copy link
Collaborator Author

Thanks for digging into this! I don't see a need for an ImageGallery at this point. I think the OrderedImage approach is fine. As it's specific for the footer with logos, we can also call it "LogoImage" then. Or perhaps even "SponsorImage", to distinguish it from the logo of the website itself - during setup of ToontjeHoger I got sometimes confused by this, that we now use "logo" both for the website logo and the logo of sponsors.

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 a pull request may close this issue.

2 participants