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.

Sign up for GitHub

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 Unique HTML IDs to .dropzones, #fileinputs #8927

Merged
merged 5 commits into from
Jan 6, 2021
Merged

Add Unique HTML IDs to .dropzones, #fileinputs #8927

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
bc302a0
generate unique HTML IDs for image upload .dropzones & inputs
noi5e Jan 3, 2021
37e4ed2
update system tests referring to #fileinput
noi5e Jan 3, 2021
9a71180
move get_dropzone_id logic to application_helper.rb #8926
noi5e Jan 5, 2021
43a66f4
change comments to HTML comments
noi5e Jan 5, 2021
12726f4
fix indents to pass linter
noi5e Jan 5, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions app/helpers/application_helper.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,18 @@ def feature_node(title)
end
end

# used in views/comments/_form.html.erb
def get_large_dropzone_id(location, reply_to)
case location
when :main
Copy link

Choose a reason for hiding this comment

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

Indent when as deep as end.

'-main'
when :reply
Copy link

Choose a reason for hiding this comment

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

Indent when as deep as end.

'-reply-' + reply_to.to_s
when :responses
Copy link

Choose a reason for hiding this comment

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

Indent when as deep as end.

'-responses'
end
end

def locale_name_pairs
I18n.available_locales.map do |locale|
[I18n.t('language', locale: locale), locale]
Expand Down
4 changes: 2 additions & 2 deletions app/views/comments/_edit.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@
#imagebar {width:100%;}
</style>


<%= render :partial => "editor/toolbar" %>
<!-- toolbar needs location & comment_id to make unique element IDs -->
<%= render :partial => "editor/toolbar", :locals => { :comment_id => comment.id.to_s, :location => :edit } %>

<div id="c<%= comment.id%>div" class="form-group">
<textarea aria-label="Edit Comment" onFocus="editing=true" name="body" class="form-control" id="c<%= comment.id%>text" rows="6" cols="40" placeholder="<%= placeholder %>"><%= !(comment.is_a?Answer) ? comment.comment : comment.content %></textarea>
Expand Down
129 changes: 66 additions & 63 deletions app/views/comments/_form.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,24 @@
padding-top: 10px;
}
</style>


<%= render :partial => "editor/toolbar" %>
<!-- location: for creating unique .dropzone IDs -->
<!-- this view is used for: main comment form, replies to comments, and 'responses' -->
<!-- if location == reply, toolbar needs reply_to for dropzone IDs -->
<% locals = (location == :reply) ?
{ :location => location, :reply_to => reply_to } :
{ :location => location }
%>
<%= render :partial => "editor/toolbar", :locals => locals %>

<% if local_assigns[:reply_to].present? %>
<%= hidden_field_tag :reply_to, local_assigns[:reply_to] %>
<% end %>

<div class="form-group dropzone">

<!-- most comment forms have two dropzones: 1) small button, 2) large form that covers textarea -->
<!-- this is the large dropzone -->
<!-- get_large_dropzone_id defined in application_helper.rb -->
<% dropzone_large_id = get_large_dropzone_id(location,local_assigns[:reply_to]) %>
<div id="dropzone-large<%= dropzone_large_id %>" class="form-group dropzone">
<%
body = body || params[:body]
# Look for comment templates
Expand All @@ -45,24 +53,20 @@
<% end %>
<textarea aria-label="Comment Text" style="border: 1px solid #bbb;border-bottom-left-radius: 0;border-bottom-right-radius: 0;border-bottom: 0;padding: 10px;" onFocus="editing=true" name="body" class="form-control" id="text-input" rows="6" cols="40" placeholder="<%= placeholder %>"><%= body %></textarea>
<div id="imagebar">

<div id="create_progress" style="display:none;" class="progress float-right">
<div id="create_progress-bar" class="progress-bar progress-bar-striped progress-bar-animated" style="width: 0%;"></div>
</div>

<p>
<span id="create_uploading" class="uploading" style="display:none;">
<%= translation('comments._form.uploading') %>
</span>
<span id="create_prompt" class="prompt">

<span style="padding-right:4px;float:left;" class="d-none d-md-inline">
<%= raw translation('comments._form.drag_and_drop') %>
</span>

<!-- http://stackoverflow.com/questions/11235206/twitter-bootstrap-form-file-element-upload-button -->
<label for="fileinput">
<input id="fileinput" type="file" name="image[photo]" style="display:none;" />
<label for="fileinput-choose-one<%= dropzone_large_id %>">
<input id="fileinput-choose-one<%= dropzone_large_id %>" type="file" name="image[photo]" style="display:none;" />
Copy link
Member

Choose a reason for hiding this comment

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

Will this change styling at all? Noting that sometimes adding unique classnames instead of ids can be better since elements are allowed to have more than one class but only one ID. I tend to use classes instead of IDs for that reason, but it's just a convention or habit, so no big deal here, just curious!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, that's a really good point about styling. I did a simple search in VSCode for #fileinput and input#fileinput but it didn't come up with anything in /assets/stylesheets, so I think this is good. It looks as normal when I load it up in development too.

That's also a good point about classes vs. IDs. Since my ultimate goal is to be able to use e.target to set $D.selected upon file upload, I thought an ID made the most sense here... Good reminder though that I can use classes in other situations.

I was aware of the one ID per page rule. dropzone_large_id includes the comment ID, so it's going to resolve to a unique ID every time this form is rendered (#fileinput-reply-large-22 or something like that)... Which means that there shouldn't be any conflicting IDs here, at least throughout comments-list!

<a onClick="handleClick()" style="cursor: pointer;" class="d-none d-md-inline text-underline">choose one</a>
<span class="d-md-none">
<i class="fa fa-upload"></i>
Expand All @@ -71,61 +75,60 @@
</label>
</span>
</p>
</div>
</div>

<div id="preview" style="background: white; display: none;">
</div>
<script>
jQuery(document).ready(function() {
$E.initialize();
});
$D = {
uid: <%= current_user.uid if current_user %>,
type: 'comment'
};

function handleClick() {
$D.selected = $(event.target.closest('div.comment-form-wrapper'));
console.log($D.selected);
$E.refresh();
}

</div>
</div>

<div id="preview" style="background: white; display: none;">
</div>
<script>
jQuery(document).ready(function() {
$E.initialize();
});
$D = {
uid: <%= current_user.uid if current_user %>,
type: 'comment'
};

function handleClick() {
$D.selected = $(event.target.closest('div.comment-form-wrapper'));
console.log($D.selected);
$E.refresh();
}

$('#input_label').click(function(e){
$E.initialize({});
});
</script>

<%= javascript_include_tag "dragdrop" %>
<script src="/emoji.js" type="text/javascript"></script>
<script src="/assets/atwho_autocomplete.js" type="text/javascript"></script>
<%= javascript_include_tag "comment.js" %>

<div class="control-group">
<button type="submit" class="btn btn-primary"><%= translation('comments._form.publish') %></button>
<% if local_assigns[:comment] %>
<a class="btn btn-outline-secondary preview-btn" onClick="$('#c<%= comment.cid %>preview').toggle();
$('#c<%= comment.cid %>text-input').toggle();
$('#c<%= comment.cid %>edit .preview-btn').button('toggle');
$E.generate_preview('c<%= comment.cid %>preview',$('#c<%= comment.cid %>text-input').val())">
<%= translation('comments._form.preview') %>
</a>
<a class="btn btn-outline-secondary" onClick="$('#c<%= comment.cid %>show').toggle();$('#c<%= comment.cid %>edit').toggle()"><%= translation('comments._form.cancel') %></a>
<% else %>
<a class="btn btn-outline-secondary preview-btn>" id="post_comment" data-previewing-text="Hide Preview" onClick="handleClick();$E.toggle_preview()"><%= translation('comments._form.preview') %></a>
<% end %>
$('#input_label').click(function(e){
$E.initialize({});
});
</script>

<%= javascript_include_tag "dragdrop" %>
<script src="/emoji.js" type="text/javascript"></script>
<script src="/assets/atwho_autocomplete.js" type="text/javascript"></script>
<%= javascript_include_tag "comment.js" %>

<div class="control-group">
<button type="submit" class="btn btn-primary"><%= translation('comments._form.publish') %></button>
<% if local_assigns[:comment] %>
<a class="btn btn-outline-secondary preview-btn" onClick="$('#c<%= comment.cid %>preview').toggle();
$('#c<%= comment.cid %>text-input').toggle();
$('#c<%= comment.cid %>edit .preview-btn').button('toggle');
$E.generate_preview('c<%= comment.cid %>preview',$('#c<%= comment.cid %>text-input').val())">
<%= translation('comments._form.preview') %>
</a>
<a class="btn btn-outline-secondary" onClick="$('#c<%= comment.cid %>show').toggle();$('#c<%= comment.cid %>edit').toggle()"><%= translation('comments._form.cancel') %></a>
<% else %>
<a class="btn btn-outline-secondary preview-btn>" id="post_comment" data-previewing-text="Hide Preview" onClick="handleClick();$E.toggle_preview()"><%= translation('comments._form.preview') %></a>
<% end %>

<span style="color:#888;"> &nbsp;
<br class="d-md-none" /><%= raw translation('comments._form.logged_in', :username => current_user.username) %> &nbsp;
<a aria-label="Authoring Help" target="_blank" href="/wiki/authoring-help#Formatting"><i class="fa fa-question-circle"></i></a> &nbsp;
<a onClick="changeNotificationIcon('#who-is-notified-form', '#bell')"><i id="bell" class="fa fa-bell-o"></i></a>
</span>
</div>
<span style="color:#888;"> &nbsp;
<br class="d-md-none" /><%= raw translation('comments._form.logged_in', :username => current_user.username) %> &nbsp;
<a aria-label="Authoring Help" target="_blank" href="/wiki/authoring-help#Formatting"><i class="fa fa-question-circle"></i></a> &nbsp;
<a onClick="changeNotificationIcon('#who-is-notified-form', '#bell')"><i id="bell" class="fa fa-bell-o"></i></a>
</span>
</div>

<p id="who-is-notified-form" style="display:none;color:#888;">
<%= translation('comments._form.email_notifications') %>
</p>
<p id="who-is-notified-form" style="display:none;color:#888;">
<%= translation('comments._form.email_notifications') %>
</p>
</form>
</div>
</div>
3 changes: 2 additions & 1 deletion app/views/editor/_editor.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<%= render :partial => "editor/toolbar" %>
<%# toolbar needs location & comment_id to make unique element IDs%>
<%= render :partial => "editor/toolbar", :locals => { :location => :main } %>

<div class="form-group dropzone">
<textarea aria-label="Wiki Text" name="body" tabindex="2" class="form-control" id="text-input" rows="20" cols="60"><% if @node && @node.latest && @node.latest.body %><%= @node.latest.body %><% else %><%= params[:body] %><% end %></textarea>
Expand Down
17 changes: 14 additions & 3 deletions app/views/editor/_toolbar.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,21 @@
<a data-toggle="tooltip" title="Header" data-placement="bottom" class="btn btn-outline-secondary btn-sm " onClick="$E.h2()"><b><span class="d-md-none d-lg-inline"><i class="fa fa-header"></i></span></b></a>
<a aria-label="Make a link" data-toggle="tooltip" title="Make a link" data-placement="bottom" class="btn btn-outline-secondary btn-sm" href="javascript:void(0)" onClick="$E.link()"><i class="fa fa-link"></i></a>
<a aria-label="Upload an image" data-toggle="tooltip" title="Upload an image" data-placement="bottom" class="btn btn-outline-secondary btn-sm">
<div class="dropzone">
<% dropzone_small_id = '' %>
<% case location
when :main %>
<% dropzone_small_id = 'main' %>
<% when :reply %>
<% dropzone_small_id = 'reply-' + local_assigns[:reply_to].to_s %>
<% when :edit %>
<% dropzone_small_id = 'edit-' + comment_id %>
<% when :responses %>
<% dropzone_small_id = 'responses' %>
<% end %>
<div id="dropzone-small-<%= dropzone_small_id %>" class="dropzone">
<span id="create_prompt" class="prompt">
<label class="" for="fileinput" style="margin-bottom: 0;">
<input id="fileinput" type="file" name="image[photo]" accept="image/*" style="display:none;" />
<label class="" for="fileinput-button-<%= dropzone_small_id %>" style="margin-bottom: 0;">
<input id="fileinput-button-<%= dropzone_small_id %>" class="fileinput" type="file" name="image[photo]" accept="image/*" style="display:none;" />
<i class="fa fa-image"></i>
<span class="d-md-none">
<i class="fa fa-upload"></i>
Expand Down
11 changes: 10 additions & 1 deletion app/views/notes/_comment.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,16 @@
<div id="comment-<%= comment.cid %>-reply-section">
<% if current_user %>
<div class="inline" id="question-comment-form">
<%= render :partial => "comments/form", :locals => { title: "Reply to this comment", reply_to: comment.cid, comment: false, placeholder: "Help the author refine their post, or point them at other helpful information on the site. Mention users by @username to notify them of this thread by email", url1: '/conduct', author: current_user.title, is_new_contributor:current_user.is_new_contributor? } %>
<%= render :partial => "comments/form", :locals => {
Copy link
Member

Choose a reason for hiding this comment

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

so much more readable, thank you!

title: "Reply to this comment",
reply_to: comment.cid,
comment: false,
placeholder: "Help the author refine their post, or point them at other helpful information on the site. Mention users by @username to notify them of this thread by email",
url1: '/conduct',
author: current_user.title,
is_new_contributor:current_user.is_new_contributor?,
location: :reply # toolbar needs this to create dropzone ID
} %>
</div>
<% else %>
<p><%= link_to "Log in", "/login?return_to=#{request.path}" %> to comment</p>
Expand Down
10 changes: 9 additions & 1 deletion app/views/notes/_comments.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,15 @@
</div>

<% if current_user %>
<%= render :partial => "comments/form", :locals => { title: I18n.t('notes._comments.post_comment'), comment: false, placeholder: I18n.t('notes._comments.post_placeholder'), url1: '/conduct', author: current_user.title, is_new_contributor:current_user.is_new_contributor? } %>
<%= render :partial => "comments/form", :locals => {
title: I18n.t('notes._comments.post_comment'),
comment: false,
placeholder: I18n.t('notes._comments.post_placeholder'),
url1: '/conduct',
author: current_user.title,
is_new_contributor:current_user.is_new_contributor?,
location: :main # toolbar needs this to create dropzone ID
} %>
<% else %>
<p><a href="#" data-toggle="modal" data-target="#loginModal">
<%= translation('layout._header.login.login_title') %> </a> to comment.
Expand Down
12 changes: 10 additions & 2 deletions app/views/notes/_responses.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,16 @@

<div class='activity-comment' id='i-did-this' style='display: none'>
<% if current_user %>
<%= render :partial => "comments/form", :locals => { title: 'Add feedback, suggestions, or photos
', body: 'I did this!', comment: false, placeholder: '', url1: '/conduct', author: current_user.title, is_new_contributor:current_user.is_new_contributor? } %>
<%= render :partial => "comments/form", :locals => {
title: 'Add feedback, suggestions, or photos',
body: 'I did this!',
comment: false,
placeholder: '',
url1: '/conduct',
author: current_user.title,
is_new_contributor:current_user.is_new_contributor?,
location: :responses
} %>
<% else %>
<p><%= raw translation('notes._comments.must_be_logged_in', :url1 => new_user_session_path( return_to: request.path )) %></p>
<% end %>
Expand Down
11 changes: 10 additions & 1 deletion app/views/questions/show.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,16 @@
<div id="answer-0-comment-section">
<% if current_user %>
<div class="d-inline" id="question-comment-form">
<%= render :partial => "comments/form", :locals => { title: "Post Comment", comment: false, placeholder: "Help the author refine their post, or point them at other helpful information on the site. Mention users by @username to notify them of this thread by email", type: "question", url1: '/conduct', author: current_user.title, is_new_contributor:current_user.is_new_contributor? } %>
<%= render :partial => "comments/form", :locals => {
title: "Post Comment",
comment: false,
placeholder: "Help the author refine their post, or point them at other helpful information on the site. Mention users by @username to notify them of this thread by email",
type: "question",
url1: '/conduct',
author: current_user.title,
is_new_contributor: current_user.is_new_contributor?,
location: :main # toolbar needs this to create dropzone ID
} %>
</div>
<% else %>
<p><a data-toggle="modal" data-target="#loginModal">Log in</a> to comment</p>
Expand Down
12 changes: 6 additions & 6 deletions test/system/comment_test.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ def setup
node_name == :wiki_page ? (visit nodes(node_name).path + '/comments') : (visit nodes(node_name).path)
find("p", text: "Reply to this comment...").click()
reply_preview_button = page.all('#post_comment')[0]
fileinput_element = page.all('#fileinput')[0]
fileinput_element = page.find('#fileinput-button-main')
# Upload the image
fileinput_element.set("#{Rails.root.to_s}/public/images/pl.png")
# Wait for image upload to finish
Expand All @@ -110,17 +110,17 @@ def setup
end

test "#{page_type_string}: comment image upload by choose one" do
Capybara.ignore_hidden_elements = false
node_name == :wiki_page ? (visit nodes(node_name).path + '/comments') : (visit nodes(node_name).path)
# Open reply comment form
find("p", text: "Reply to this comment...").click()
first("a", text: "choose one").click()
reply_preview_button = page.all('#post_comment')[0]
fileinput_element = page.all('#fileinput')[0]
reply_preview_button = page.first('a', text: 'Preview')
Capybara.ignore_hidden_elements = false
# Upload the image
fileinput_element = page.first("[id^=fileinput-button-reply]")
fileinput_element.set("#{Rails.root.to_s}/public/images/pl.png")
# Wait for image upload to finish
wait_for_ajax
Capybara.ignore_hidden_elements = true
wait_for_ajax
# Toggle preview
reply_preview_button.click()
# Make sure that image has been uploaded
Expand Down