-
Couldn't load subscription status.
- Fork 45
Hide upload button from default toolbar when attachments is set to false #315
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
base: main
Are you sure you want to change the base?
Conversation
1d0b87e to
33f2c3e
Compare
test/system/toolbar_test.rb
Outdated
| test "disable attachments" do | ||
| visit edit_post_path(posts(:empty), attachments_disabled: true) | ||
|
|
||
| assert_no_selector "lexxy-toolbar button[name=upload]" | ||
| end |
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.
Could it be worth testing that the button is still present when false or blank?
test "enable attachments" do
visit edit_post_path(posts(:empty), attachments_disabled: [false, nil].sample)
assert_no_selector "lexxy-toolbar button[name=upload]"
endThere 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.
Sounds good. I've updated the tests.
Note: The form on the page edit_post_path(posts(:empty), attachments_disabled: true) also needed a change for the test to work. To some extent, we're more testing the query parameter than the actual attachments attribute with these new tests.
33f2c3e to
aeb835d
Compare
|
I've rebased onto |
| <%= form.label :body, style: "display: block" %> | ||
| <%= form.rich_text_area :body, placeholder: "Write something...", | ||
| attachments: params[:attachments_disabled] ? "false" : nil, | ||
| attachments: (params[:attachments_disabled] != "true"), |
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.
This is a bit annoying as this change now diverges from the other query parameter disabling option logic on L15: params[:toolbar_disabled] ? "false" : nil
Fixes #307
Based on the discussion on issue #307, this also works and could be enough?
Happy to provide a change for either approach