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

Conversation

xtine
Copy link
Member

@xtine xtine commented Apr 7, 2016

  • Read Tab link
  • Edit comment link
  • Clicking "I read and understand..." checkbox will enable submit button
  • Underline hover for PDF preview download link

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.

@jmcarp
Copy link
Contributor

jmcarp commented Apr 7, 2016

I noticed that ajax navigation to comments on cfr changes loses the toc and errors. This makes sense, since we aren't including the cfr toc in the template here. We should do that. This also adds ajax navigation from the review page to the preamble pages, but not the reverse, so the back button doesn't work. I think it's fine to save those for a separate PR, but if we do, let's file an issue so this doesn't get lost.

@cmc333333 cmc333333 merged commit 0f99613 into eregs:master Apr 8, 2016
@xtine xtine deleted the add-review-functionality branch July 7, 2016 05:51
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants