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

[WIP] Write mode polish #397

Merged
merged 10 commits into from
Jun 8, 2016
Merged

[WIP] Write mode polish #397

merged 10 commits into from
Jun 8, 2016

Conversation

xtine
Copy link
Member

@xtine xtine commented Jun 8, 2016

  • Excerpt toggle label just says "Show"
    • remove show more/less text toggles
  • Clean up nesting of comment attachment styles
    • line up "Upload Attachment" and "Save Response" buttons

Fixes: 18F/epa-notice#354

screen shot 2016-06-07 at 6 08 24 pm

@xtine xtine changed the title Write mode polish [WIP] Write mode polish Jun 8, 2016
@cmc333333
Copy link
Member

Something's wrong with my local setup, so I can't test locally yet. But! Changes look good in the abstract. Is this in a state to merge?

For future reference, most of the stylesheet changes are just moving things around. New stuff:

  • .comment-context-toggle -> a.comment-context-toggle (with new text color)
  • .comment-controls has a new nested class
  • .comment-submission
  • .comment-upload-button
  • .comment-attachment margin, last-child
  • .comment-submission

@xtine
Copy link
Member Author

xtine commented Jun 8, 2016

Multiple comment attachments are now stacked vertically and also accounts for extra long file names.

screen shot 2016-06-08 at 11 42 52 am

@donjo
Copy link
Member

donjo commented Jun 8, 2016

Just confirmed a few things with @jehlers:

  • The actual cited text should stay black. The only part that needed to be gray (#767676) was what was in the gray bar.
  • The bar should change from "Show" to "Hide" when it's expanded.

@xtine
Copy link
Member Author

xtine commented Jun 8, 2016

screen shot 2016-06-08 at 12 56 01 pm

@donjo
Copy link
Member

donjo commented Jun 8, 2016

Looks good. Is the bug in 18F/epa-notice#367 related to this in any way or is it ready to merge?

@@ -116,6 +118,8 @@ var CommentView = Backbone.View.extend({

this.$contextSectionLabel.html(options.label);

this.$deleteResponseDiv.attr('data-section', options.section);
Copy link
Member

Choose a reason for hiding this comment

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

This works, but we should probably switch to a template at some point

@xtine
Copy link
Member Author

xtine commented Jun 8, 2016

The comment delete/save bug wasn't triggered with this PR.

@cmc333333
Copy link
Member

cmc333333 commented Jun 8, 2016

General todo, we have lots and lots of e.preventDefault();'s now, which is concerning. I don't think we need to address that right now, however. Added #401 for this. I've not tested this locally, but if it looks good to you, go for the merge @donjo

@donjo donjo merged commit e0cc87a into eregs:master Jun 8, 2016
@xtine xtine deleted the 354-write-mode-polish 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