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

Org Page Content Epic: As staff I can upload a Hero Image and up to two About Us images #592

Closed
kasugaijin opened this issue Mar 25, 2024 · 14 comments
Assignees

Comments

@kasugaijin
Copy link
Collaborator

kasugaijin commented Mar 25, 2024

Currently blocked by #480

As staff, you can go to the dashboard and see the Page Text navbar option, where you can update the Hero Text and About Us text. This also needs to allow staff to update their Hero Image (1 image) and About Us Images (1 or 2 images) on the org's root/home page.

Implementation Criteria:

  • Use the PageText model for the attachments: has_one_attached :hero_imageand has_many_attached :about_us_image
  • On the PageText form add attachment inputs, clearly labelled. One for Hero Image (allows one attachment), One for About Us images (allows one or two attachments).
  • Add model validations to ensure the files are images, not massive (see the Pets Controller for example)
  • Add controller actions on PageTextsController for each type of attachment to attach (can use existing attachments controller for purge)
  • Staff should be able to upload and purge (delete) an image.
  • Resuse partials where it makes sense e.g., the upload input, and the table to show the uploaded images (see the partials used in the Pet tabs for photos and files)
  • Add permissions to the policy (may not be needed based on what is added in Org Page Content Epic: As org staff I can go to the dashboard and input text that will update the hero and about us Part 2 #480).
  • On org root/home, render the Org.page_text.hero_image if it exists, and render either one or both About Us images, if they exist. If there is only one image, then use an existing default image for the second.
  • Tests

As next steps (different issue) we will want to look into recommending image dimensions and handling resizing of image variants. Feel free to include in this issue if you want to.

I may have forgotten something here - hit me up in Slack!

@ErinClaudio
Copy link
Collaborator

Can I give this one a crack? @kasugaijin
If yes I would like to write a breakdown of my path forward before beginning any coding.

@kasugaijin
Copy link
Collaborator Author

kasugaijin commented Mar 30, 2024

@ErinClaudio yes sounds good!

This ticket is actually blocked by this one #552 but PR is ready for review so if you want to wait then all good

@ErinClaudio ErinClaudio self-assigned this Apr 1, 2024
@ErinClaudio
Copy link
Collaborator

@kasugaijin

For clarification, the functionality for updating the Hero Image and About Us Images should be integrated directly within the 'Page Text' navbar option, as opposed to introducing a separate button?

@ErinClaudio
Copy link
Collaborator

ErinClaudio commented Apr 1, 2024

@kasugaijin
Copy link
Collaborator Author

@kasugaijin

For clarification, the functionality for updating the Hero Image and About Us Images should be integrated directly within the 'Page Text' navbar option, as opposed to introducing a separate button?

Yes I should be able to click Page Text in the sidebar if the dashboard and then see the form to edit text and attach images.

@kasugaijin
Copy link
Collaborator Author

My notes for the issue https://gist.github.com/ErinClaudio/393ca48b7f2a77b8d65d5ab36a7f120c

Yes looks good! You might run into questions on some details along the way so hit us up as usual! Thanks @ErinClaudio and let me know if anything is unclear before diving in so as to save you any headaches

@ErinClaudio
Copy link
Collaborator

@kasugaijin @mononoken @nsiwnf
Screenshot 2024-04-02 at 12 14 20 PM

Here's my progress so far. However, I don't believe it's correct. I think one of the following approaches would be better:

  1. Create a separate but similar form specifically for images to simplify the controller logic.
  2. Add a second submit button within the existing form for image uploads, with differentiation handled by the controller.

I lean towards the first option but am open to either.

@kasugaijin
Copy link
Collaborator Author

@ErinClaudio This looks ok to me? Then we can display each attached image in a table below the submit button to show current attachments. What do you not like about this current setup?

Two submit buttons on a form can be problematic. The first option of a separate form would be easier to maintain I think. I am also not opposed to going that route, either., But, since the information is directly related in this form you have currently, it makes sense to me to keep it like this. Controller logic shouldn't be too different? I think we just need to accept appended images in the accepted params?

@ErinClaudio
Copy link
Collaborator

@kasugaijin

This looks horrible. But just wanted to show progress, I have the images rendering. not correctly but I will fix that.
More work to be done.

Screenshot 2024-04-03 at 2 05 35 PM

@kasugaijin
Copy link
Collaborator Author

@ErinClaudio haha nice cow :)

@ErinClaudio
Copy link
Collaborator

@kasugaijin This issue is Mooooooooving along

@ErinClaudio
Copy link
Collaborator

@kasugaijin @nsiwnf @mononoken

I could use a little feedback.

The delete button layout feels cluttered. Consider:

  1. A single button to delete both image types.
  2. Separate buttons for each image type, positioned at the end of their respective form fields for clarity.
Screenshot 2024-04-04 at 11 03 46 AM

@kasugaijin
Copy link
Collaborator Author

kasugaijin commented Apr 4, 2024

@ErinClaudio could we re-use the partials for image uploads - place them where the existing upload inputs are in your UI.

Then, under the submit button, reuse the table partial to display uploaded images?

See Pet Images tab for reference

image

@ErinClaudio
Copy link
Collaborator

@kasugaijin
Perfect and thanks

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

No branches or pull requests

2 participants