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

Background image for Collection and Category #1882

Merged
merged 20 commits into from
Mar 20, 2018

Conversation

Pacu2
Copy link
Contributor

@Pacu2 Pacu2 commented Mar 7, 2018

In this pull request I've added ability to upload background image for Categories and Collections.
Moreover, adequate images are added to Categories and Collections created by running populatedb command.
Collections are more organized, created from schema, without randomly generated names.

Mobile

saleor_category_mobile

Desktop

saleor_category_laptop

TODO:

  • Random images in populatedb
  • Tests for image upload

Pull Request Checklist

(Please keep this section. It will make maintainer's life easier.)

  1. Privileged views and APIs are guarded by proper permission checks.
  2. All visible strings are translated with proper context.
  3. All data-formatting is locale-aware (dates, numbers, and so on).
  4. Database queries are optimized and the number of queries is constant.
  5. The changes are tested.
  6. The code is documented (docstrings, project documentation).
  7. Python code quality checks pass: pycodestyle, pydocstyle, pylint.
  8. JavaScript code quality checks pass: eslint.

@Pacu2 Pacu2 self-assigned this Mar 7, 2018
@Pacu2
Copy link
Contributor Author

Pacu2 commented Mar 9, 2018

Waiting for images for populatedb

@Pacu2 Pacu2 force-pushed the feature/category-collection-background-image branch from 44e6d5d to 9b7a1e5 Compare March 16, 2018 09:10
@Pacu2 Pacu2 removed the in progress label Mar 16, 2018
@codecov
Copy link

codecov bot commented Mar 16, 2018

Codecov Report

❗ No coverage uploaded for pull request base (master@db3c546). Click here to learn what that means.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #1882   +/-   ##
=========================================
  Coverage          ?   84.55%           
=========================================
  Files             ?      160           
  Lines             ?     6871           
  Branches          ?      704           
=========================================
  Hits              ?     5810           
  Misses            ?      870           
  Partials          ?      191
Impacted Files Coverage Δ
saleor/dashboard/collection/forms.py 100% <ø> (ø)
saleor/product/models.py 94.46% <100%> (ø)
saleor/dashboard/collection/views.py 89.47% <100%> (ø)
saleor/dashboard/category/views.py 82.75% <100%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update db3c546...6c3b9cf. Read the comment docs.

@Pacu2 Pacu2 changed the title [WIP] background image for Collection and Category Background image for Collection and Category Mar 16, 2018
return product_list_images_dir


def get_product_list_image(image_dir, image_name):
Copy link
Contributor

Choose a reason for hiding this comment

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

This name could be more general, as it's functionality is just to open a file.

@@ -29,6 +29,8 @@ class Category(MPTTModel):
parent = models.ForeignKey(
'self', null=True, blank=True, related_name='children',
on_delete=models.CASCADE)
background_image = VersatileImageField(
upload_to='category_backgrounds', blank=True, null=True)
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we rather use hyphen here? Similar problem was mentioned in #1553. Same below.

@@ -98,6 +98,12 @@ <h3 class="d-md-none float-left filters-toggle">
</div>
{% endblock %}

{% block topcontent %}
{% if object.background_image %}
<div class="row" id="product-list-image" style="background-image: url('{{ object.background_image.url }}')"></div>
Copy link
Member

Choose a reason for hiding this comment

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

What do you think about making sure that the image is not too big (mainly dimensions)?
For example, if we have a staff member that would upload unoptimized pictures, like a picture taken from a mobile camera.

Currently .container has a max-width of 1140px, maybe we could do something to prevent the page from being too data heavy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As an enhancement, background_image should be generated in various sizes, same as product images, so I'd not bother for now

Copy link
Member

Choose a reason for hiding this comment

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

Okay then 👍

@Pacu2 Pacu2 force-pushed the feature/category-collection-background-image branch from 4d27eee to 56024b0 Compare March 20, 2018 10:59
@Pacu2 Pacu2 force-pushed the feature/category-collection-background-image branch from 56024b0 to 822cb4b Compare March 20, 2018 11: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.

None yet

4 participants