Skip to content
This repository has been archived by the owner on Apr 4, 2023. It is now read-only.

Add Review page functionality #207

Merged
merged 6 commits into from
Apr 8, 2016
Merged

Add Review page functionality #207

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
d37f222
janky way for read tab to work in review page
xtine Apr 6, 2016
a72b666
review page: better way to go back to read preamble
xtine Apr 7, 2016
c7543f1
review page: edit comment links
xtine Apr 7, 2016
02e0f47
agree checkbox toggle submit button
xtine Apr 7, 2016
5068734
Merge remote-tracking branch 'upstream/master' into add-review-functi…
xtine Apr 8, 2016
a392c9c
no need to add preamble-wrapper class
xtine Apr 8, 2016
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
17 changes: 15 additions & 2 deletions regulations/static/regulations/css/less/module/comment.less
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,11 @@ a.comment-index-review {
line-height: 18px;
text-transform: inherit;
width: 425px;

&:hover {
cursor: pointer;
text-decoration: underline;
}
}

.details {
Expand Down Expand Up @@ -373,13 +378,21 @@ a.comment-index-review {
}

.submit-button {
background: @grey;
background: @pacific;
border-radius: 3px;
color: #000;
color: #FFF;
font-size: 17px;
font-weight: bold;
height: auto;
padding: 12px 30px;

&:disabled, &:disabled:hover {
background: @grey;
color: #000;
}
&:hover {
background-color: @pacific_hover;
}
}
}

Expand Down
32 changes: 30 additions & 2 deletions regulations/static/regulations/js/source/views/comment/comment-review-view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,16 @@ var _ = require('underscore');
var Backbone = require('backbone');
Backbone.$ = $;

var comments = require('../../collections/comment-collection');

var MainEvents = require('../../events/main-events');
var PreambleHeadView = require('../header/preamble-head-view');
var CommentEvents = require('../../events/comment-events');
var comments = require('../../collections/comment-collection');

var CommentReviewView = Backbone.View.extend({
events: {
'click .edit-comment': 'editComment',
'click .preview-button': 'preview',
'change .agree': 'toggleSubmit',
'click .submit-button': 'submit'
},

Expand All @@ -27,19 +29,41 @@ var CommentReviewView = Backbone.View.extend({

this.previewLoading = false;

this.listenTo(CommentEvents, 'read:proposal', this.handleRead);

this.render();
},

findElms: function() {
this.$status = this.$el.find('.status');
},

handleRead: function() {
var section = this.docId + '-preamble-' + this.docId + '-I';
var options = {id: section, section: section, mode: 'read'};

$('#content-body').removeClass('comment-review-wrapper').addClass('preamble-wrapper');
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to add the preamble-wrapper class here? I don't see references to it in the less or js. Maybe we can drop that class altogether. Also, if we moved the comment-review-wrapper class from the div to the child section, would we have to remove the class at all?

Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't double check that .preamble-wrapper was being used, but .comment-review-wrapper does need to be on the same element as #content-body to resize the width of the content area.


MainEvents.trigger('section:open', section, options, 'preamble-section');
},

editComment: function(e) {
var section = $(e.target).closest('li').data('section');
var options = {id: section, section: section, mode: 'write'};

$('#content-body').removeClass('comment-review-wrapper').addClass('preamble-wrapper');

MainEvents.trigger('section:open', section, options, 'preamble-section');
CommentEvents.trigger('comment:writeTabOpen');
},

render: function() {
var commentData = comments.toJSON({docId: this.docId});
var html = this.template({
comments: commentData,
previewLoading: this.previewLoading
});

this.$content.html(html);
this.findElms();

Expand Down Expand Up @@ -72,6 +96,10 @@ var CommentReviewView = Backbone.View.extend({
this.render();
},

toggleSubmit: function() {
$('.submit-button').prop('disabled', function(i, v) { return !v; });
Copy link
Contributor

Choose a reason for hiding this comment

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

Procedural updates like this might not mix well with the template rendering we're doing elsewhere in this view. AFAICT, If a user requests a comment download, then checks the "I agree" box, and then the download finishes, we're going to re-render the entire section, which will disable the submit button again. We could roll this update into the template, or only use the render method to update certain parts of the view.

Copy link
Member Author

Choose a reason for hiding this comment

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

Good point. If we were to update it template-side, would it be inline JS or something else..?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, I've been handling logic in templates with gross backbone template js, like <% if (true) { %>something<% } %>.

Copy link
Member

Choose a reason for hiding this comment

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

Made a separate issue for this: #214 so we can merge this PR for demo.

},

submit: function() {
var prefix = window.APP_PREFIX || '/';
var $xhr = $.ajax({
Expand Down
3 changes: 2 additions & 1 deletion regulations/static/regulations/js/source/views/header/preamble-head-view.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ var PreambleHeadView = Backbone.View.extend({
},

readProposal: function() {
if ($('#preamble-write').is(':visible')) {
// yeah... need a better way to manage this. - xtine
if ($('#preamble-write').is(':visible') || $('#comment-review').length) {
this.readTabOpen();

CommentEvents.trigger('read:proposal');
Expand Down
4 changes: 3 additions & 1 deletion regulations/templates/regulations/comment-review-chrome.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
class="drawer-toggle-icon" alt="Table of Contents toggle" /></a></li>
{% endblock %}

{% block wayfinding %}{% endblock %}

{% block header-secondary %}
{% include "regulations/preamble-header.html" %}
{% endblock %}
Expand Down Expand Up @@ -56,7 +58,7 @@ <h3>You are submitting an official comment to Regulations.gov.</h3>
<div class="comments">
<ul>
<% _.each(comments, function(comment, index) { %>
<li>
<li data-section="<%= comment.id %>">
<h4><%= comment.label %></h4>
<div class="comment-content">
<%= comment.comment %>
Expand Down
6 changes: 1 addition & 5 deletions regulations/templates/regulations/preamble-chrome.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@
<li><a href="#table-of-contents-secondary" id="table-of-contents-secondary-link" class="toc-nav-link" title="CFR Changes">CFR</a></li>
{% endblock %}

{% block wayfinding %}
<span class="subpart">{{doc_number}}</span>
<span id="active-title"></span>
{% endblock %}

{% block wayfinding %}{% endblock %}

{% block header-secondary %}
{% include "regulations/preamble-header.html" %}
Expand Down