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

We need an image Field Partial #498

Closed
jagthedrummer opened this issue Aug 30, 2023 · 0 comments · Fixed by #515
Closed

We need an image Field Partial #498

jagthedrummer opened this issue Aug 30, 2023 · 0 comments · Fixed by #515

Comments

@jagthedrummer
Copy link
Contributor

jagthedrummer commented Aug 30, 2023

Related to #464. (I think the underlying details have changed a bit since that issue was opened, and the new details point to us needing a new feature more than needing to fix a bug.)

As discussed in #464, we're currently not rendering a Profile Photo on the "Update Your Profile" page when we use ActiveStorage to handle the upload instead of Cloudinary.

CleanShot 2023-08-30 at 09 59 15

The reason that shows up like it does is that when Cloudinary is not enabled we render a simple field partial.

<% if cloudinary_enabled? %>
<%= render 'shared/fields/cloudinary_image', method: :profile_photo_id %>
<% else %>
<%= render 'shared/fields/file_field', method: :profile_photo %>
<% end %>

I think that we really need a new type of field partial that is just image. And then inside the related partials we can do the checks for whether cloudinary is enabled.

That would mean that on user form we could eliminate the conditional and just do:

<%= render 'shared/fields/image', method: :profile_photo_id %>

I think that developers for the most part shouldn't ever directly see cloudinary_enabled?, and it shouldn't be in most high-level templates.

We'll probably want to preserve the cloudinary_image field partials, for backwards compatibility, but we could probably just have them render the new image partial stuff.

jagthedrummer added a commit to bullet-train-co/bullet_train that referenced this issue Sep 1, 2023
This (mostly) fixes the problem reported in bullet-train-co/bullet_train-core#464

However, due to bullet-train-co/bullet_train-core#498 we don't currently try to show
a user profile photo if we're using ActiveStorage instead of Cloudinary.

Here's some context about why we need to add this directly to the
`Gemfile`.
libvips/ruby-vips#155 (comment)
jagthedrummer added a commit to bullet-train-co/bullet_train that referenced this issue Sep 1, 2023
* Add `ruby-vips` in the `:development, :test` group

This (mostly) fixes the problem reported in bullet-train-co/bullet_train-core#464

However, due to bullet-train-co/bullet_train-core#498 we don't currently try to show
a user profile photo if we're using ActiveStorage instead of Cloudinary.

Here's some context about why we need to add this directly to the
`Gemfile`.
libvips/ruby-vips#155 (comment)

* Try it here
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.

1 participant