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

Upload images through the markdown editor #261

Merged
merged 3 commits into from
Oct 21, 2022
Merged

Upload images through the markdown editor #261

merged 3 commits into from
Oct 21, 2022

Conversation

y-chiasson
Copy link
Contributor

@y-chiasson y-chiasson commented Jul 7, 2017

@y-chiasson
Copy link
Contributor Author

You can put the following code in a .sh file and execute it to create example app.
Then go to http://localhost:3000/admin/articles/new to try the markdown editor

rails _4.2.9_ new markdown_example_app && cd markdown_example_app
echo "gem 'fae-rails', git: 'git://github.com/buysell-technologies/fae.git', branch: 'upstream-issue#260'" >> ./Gemfile
bundle install
bundle exec rails g fae:install

rails g fae:scaffold Article title:string body:text
rake db:migrate
rake fae:seed_db

cat <<EOF > ./app/views/admin/articles/_form.html.slim 
= simple_form_for(['admin', @item]) do |f|
  header.content-header.js-content-header
    == render 'fae/shared/form_header', header: @klass_name
  == render 'fae/shared/errors'

  main.content
    = fae_input f, :title
    = fae_input f, :body, as: :text, markdown: true

EOF

# rails s
# http://localhost:3000/admin/articles/new

Copy link
Member

@jamesmk jamesmk left a comment

Choose a reason for hiding this comment

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

@y-chiasson Thanks for the PR! I like this feature and am interested in merging it. Can you find somewhere to document the feature, since there's no implementation requirements it can be brief. also, can you write a couple tests for it?

@y-chiasson
Copy link
Contributor Author

@jamesmk Thank you for having a look at this PR. I will commit some documentation and tests as soon as possible!

@christophebeling
Copy link

How is the progress here? We could really need this feature! If there is something i could help with feel free to get in touch with we :)

@jamesmk
Copy link
Member

jamesmk commented Oct 3, 2017

@christophebeling This feature is missing documentation and tests. If you are interested in contributing @y-chiasson might accept a PR into their upstream-issue#260 branch?

@christophebeling
Copy link

@jamesmk what kind of tests did you think of? do you use any javascript testing framework yet? documentation dosen't look that tough since this feature is pretty easy to use. but testing all the new javascript might be a lot of work

@jamesmk
Copy link
Member

jamesmk commented Oct 27, 2017

@christophebeling We don't have a dedicated JS testing frameworks, but you can test JS interactions via our acceptance testing framework, Capybara. I'm looking for a test emulating a successful interaction with the feature.

We don't use the WYSIWYG internally, so we rely on tests to keep the less visible features stable. The more the merrier :)

@johnnyshields
Copy link
Contributor

👍 for getting this in the next release

@johnnyshields
Copy link
Contributor

What's the status of this?

@abejosua
Copy link

How can I use this?

@jamesmk
Copy link
Member

jamesmk commented Apr 5, 2018

@johnnyshields I have requested some tests along with the PR. As soon as someone adds them, I'll be happy to merge.

@abejosua You can point Fae to @y-chiasson's branch in your Gemfile. However, you will no longer receive bugfixes and features from the main Gem. Bonus points: you could fork it and adds tests to this PR and then it will be a part of Fae core ;)

@johnnyshields
Copy link
Contributor

Can someone at FINE get this to the finish line? We've been using it in production for half a year without issue.

@jsonjordan jsonjordan changed the base branch from master to 94839_give_credit_add_tests October 18, 2022 14:23
@jsonjordan jsonjordan merged commit e2f2126 into wearefine:94839_give_credit_add_tests Oct 21, 2022
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.

6 participants