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

Fix CKEditor height in dashboard actions form #3641

Merged
merged 1 commit into from
Sep 4, 2019
Merged

Conversation

javierm
Copy link
Member

@javierm javierm commented Jun 24, 2019

References

The following Travis builds failed due to this problem:

Objectives

  • Improve the dashboard actions form usability
  • Fix flaky specs caused by this issue

Notes

Not wrapping the editor in a .ckeditor div made it change height when the editor was loaded. That caused a weird effect for users, and also made some tests fail sometimes since the position of the "Add new document" link might change right when capybara is clicking it.

These specs have failed because of this issue:

  1) Admin dashboard actions behaves like nested documentable at new_admin_dashboard_action_path Should update document cached_attachment field after valid file upload
     Failure/Error: document_input = document.find("input[type=file]", visible: false)
     NoMethodError:
       undefined method `find' for nil:NilClass
     Shared Example Group: "nested documentable" called from ./spec/features/admin/dashboard/actions_spec.rb:10
     # ./spec/shared/features/nested_documentable.rb:330:in `documentable_attach_new_file'
     # ./spec/shared/features/nested_documentable.rb:151:in `block (3 levels) in <top (required)>'
  1) Admin dashboard actions behaves like nested documentable at new_admin_dashboard_action_path Should show document errors after documentable submit with
              empty document fields
     Failure/Error:
       within "#nested-documents .document" do
         expect(page).to have_content("can't be blank", count: 2)
       end
     Capybara::ElementNotFound:
       Unable to find visible css "#nested-documents .document"
     Shared Example Group: "nested documentable" called from ./spec/features/admin/dashboard/actions_spec.rb:10
     # ./spec/shared/features/nested_documentable.rb:176:in `block (3 levels) in <top (required)>'
  1) Admin dashboard actions behaves like nested documentable at new_admin_dashboard_action_path Should not update document cached_attachment field after invalid file upload
     Failure/Error: document_input = document.find("input[type=file]", visible: false)
     NoMethodError:
       undefined method `find' for nil:NilClass
     Shared Example Group: "nested documentable" called from ./spec/features/admin/dashboard/actions_spec.rb:10
     # ./spec/shared/features/nested_documentable.rb:330:in `documentable_attach_new_file'
     # ./spec/shared/features/nested_documentable.rb:160:in `block (3 levels) in <top (required)>'

Not wrapping the editor in a `.ckeditor` div made it change height when
the editor was loaded. That caused a weird effect for users, and also
made some tests fail sometimes since the position of the "Add new
document" link might change right when capybara is clicking it.
@javierm javierm merged commit 6923cdd into master Sep 4, 2019
@javierm javierm deleted the fix_ckeditor_height branch September 4, 2019 12:59
@javierm javierm added this to Release 1.1.0 in Roadmap Sep 10, 2019
smarques pushed a commit to venetochevogliamo/consul that referenced this pull request Apr 29, 2020
Fix CKEditor height in dashboard actions form
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Roadmap
  
Release 1.1.0
Development

Successfully merging this pull request may close these issues.

None yet

2 participants