-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 RichText field type #2411
base: main
Are you sure you want to change the base?
Add RichText field type #2411
Conversation
Will look into the failing tests, some of them are not failing for me locally :( |
I had a look. It appears to be order-dependent. Specifically, when I run en example from administrate/spec/support/generator_spec_helpers.rb Lines 38 to 41 in 852b62c
So it looks that, whatever happens there, it deletes the Thoughts? |
Are we planning to release this anytime soon - Need this badly for actiontext |
@littleforest, are you able to pick up from @seanpdoyle's comments here? (If not, I can do so!) |
@nickcharlton I will see if I can find time to work on this second week of January. Might go faster if I can find time to pair with you or @seanpdoyle . |
Copied the trix.js file into trix_custom.js and removed the export line at the end for the Sprockets implementation. Not sure if there is a cleaner way to handle this.
@littleforest, sorry, only getting back to this one now. How did you get on? Feel free to drop some time on my calendar if you've like to pair on this. |
@littleforest the asset changes merged as part of #2397 have introduced merge conflicts. If you're interested, I'm willing to give resolving those conflicts a try and force-pushing to this branch. If not, and you'd rather handle that yourself, that's great too! |
That would be amazing! Thank you. |
@seanpdoyle @nickcharlton @pablobm this is ready for re-review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
Just one thought on added dependencies, but we should be able to merge this shortly.
@@ -7,6 +7,7 @@ gem "administrate-field-image" | |||
gem "faker" | |||
gem "front_matter_parser" | |||
gem "globalid" | |||
gem "image_processing" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean we'll now have a system dependency on ImageMagick
or libvips
?
I guess if you're doing this with Rails that's going to happen anyway, so maybe it's not a concern for us?
@@ -37,7 +37,7 @@ def each_file_in(path) | |||
|
|||
def reset_routes | |||
Rails.application.routes.clear! | |||
load "spec/example_app/config/routes.rb" | |||
Rails.application.routes_reloader.execute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooh, that's a nice change from just loading the file!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I needed this since there were some active storage direct upload URLs that didn't get reset without it.
This adds a RichText field type, completing the work started in #1660.