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

Jump to bottom

ui patch agent show #1326

Merged
merged 17 commits into from
Mar 12, 2016
Merged

ui patch agent show #1326

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
7fd6ff3
ui patch agent show
Mar 2, 2016
6f46da0
switched to toggle jquery method
Mar 3, 2016
fdbb055
attempt to convert js to coffee
Mar 3, 2016
e2f2f47
attempt to convert js to coffee
Mar 3, 2016
e6b96b1
added in another code option
Mar 3, 2016
caa4c3e
done got it to work
Mar 3, 2016
c96e402
rm code from view and indent properly
Mar 3, 2016
6e26e9f
js code show page
Mar 3, 2016
7a9b405
made button text dependent updated jquery
Mar 4, 2016
e86045c
defining js code a bit down
Mar 4, 2016
f3209c0
rewrote code into an instance property and added it to the constructor
Mar 4, 2016
61b8e0a
rm unnecessary e params into method
Mar 4, 2016
b7f906e
fixed spacing
Mar 4, 2016
60db396
last fix to spacing
Mar 4, 2016
6b212c8
fixing html related errors
Mar 7, 2016
508fe28
made button text dynamic with jquery text
Mar 7, 2016
b226a78
unnecessary change
Mar 7, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 8 additions & 1 deletion app/assets/javascripts/pages/agent-show-page.js.coffee
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
$('#target').click ->
$('#memorytoggle').toggleClass 'hidden'
return

`$( "#target" ).click(function() {
$("#memorytoggle").toggleClass("hidden");
});`

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The code does not work here even though I tried both passing in a pure js and converted it to coffee. However it works in show.html.erb.

class @AgentShowPage
constructor: ->
$(".agent-show #show-tabs a[href='#logs'], #logs .refresh").on "click", @fetchLogs
Expand Down Expand Up @@ -56,4 +64,3 @@ class @AgentShowPage

$ ->
Utils.registerPage(AgentShowPage, forPathsMatching: /^agents\/\d+/)

27 changes: 18 additions & 9 deletions app/views/agents/show.html.erb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,17 +161,26 @@
<b>Options:</b>
<pre><%= Utils.pretty_jsonify @agent.options || {} %></pre>
</p>

<p id="memory" data-agent-id="<%= @agent.id %>">
<b>Memory:</b>
<% if @agent.memory.present? %>
<i class="fa fa-spinner fa-pulse spinner"></i>
<i class="fa fa-trash action-icon clear"></i>
<% end %>
<pre class="memory"><%= Utils.pretty_jsonify @agent.memory || {} %></pre>
</p>
<button type="submit" class="btn btn-default" id="target">Hide or Reveal Memory</button>
Copy link
Member

Choose a reason for hiding this comment

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

Can you rename target to something more descriptive, like toggle-memory? The text could be "Show Memory" and the CoffeeScript above could be

$('#toggle-memory').click ->
  if $('#toggle-memory').text() == 'Show Memory"
    $('#toggle-memory').text("Hide Memory")
    $('#memory').removeClass 'hidden'
  else
    $('#toggle-memory').text("Show Memory")
    $('#memory').addClass 'hidden'

Finally, you don't need the memorytoggle div, since you can target #memory directly, just add the hidden class to it to start.

Copy link
Member

Choose a reason for hiding this comment

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

If you use the new, longer CoffeeScript though, define it further down in the file like clearMemory and bind it at the top.

Copy link
Collaborator

Choose a reason for hiding this comment

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

with if $('#memory').hasClass('hidden') you could make it dependent from the shown text 😄

<div id="memorytoggle" class="hidden">
<p id="memory" data-agent-id="<%= @agent.id %>">
<b>Memory:</b>
<% if @agent.memory.present? %>
<i class="fa fa-spinner fa-pulse spinner"></i>
<i class="fa fa-trash action-icon clear"></i>
<% end %>
<pre class="memory"><%= Utils.pretty_jsonify @agent.memory || {} %></pre>
</p>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

You added one opening div and two closing /divs, I think one of the closing divs needs to be removed.

</div>
</div>
</div>
</div>
</div>


<%#= javascript_tag do %>
<!-- $( "#target" ).click(function() {
$("#memorytoggle").toggleClass("hidden");
}); -->
<%#end %>