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

add posibility to add image preview in show/edit page #161

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

sulphur
Copy link

@sulphur sulphur commented Oct 20, 2020

No description provided.

@aesmail
Copy link
Owner

aesmail commented Oct 20, 2020

Thank you @sulphur
This is definitely a good PR. I'd change some minor things though.

  1. Instead of checking for the type inside the form_field function, you might want to move that inside the build_html_input function where all the types are already being checked and the function returns the necessary html.
    Check this line.

  2. For the new type names, image_preview and custom_html are good, but we can make them shorter like image and html. Less typing, and meaning is preserved.

I think this PR is ready to be merged once this is done 👍🏼

@sulphur
Copy link
Author

sulphur commented Dec 3, 2020

Hi, Sure I can change it to html and image. However I just wanted to point out before that html and image aren't form inputs that is why I didn't put them inside buitd_html_input since this function creates form elements. Tell me what you think and I will update the PR accordingly.

@kelcecil kelcecil added the enhancement New feature or request label Sep 20, 2021
@artempankov
Copy link

is there any chance for merging this request? ))

@Faymir
Copy link

Faymir commented Sep 5, 2022

Hello, any news about this pull request ?

@aesmail
Copy link
Owner

aesmail commented Sep 5, 2022

I will try to merge this for the next release 👍🏻

@ghenry
Copy link
Collaborator

ghenry commented Apr 12, 2023

Hi @sulphur

Would you be so kind as to do this again to the master branch?

Thanks,
Gavin.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants