Skip to content
This repository has been archived by the owner on Oct 2, 2019. It is now read-only.

86661892 notifications include comments #203

Merged
merged 7 commits into from
Feb 6, 2015

Conversation

phirefly
Copy link
Contributor

@phirefly phirefly commented Feb 6, 2015

Added a simple display of comments to the top of notification emails. Will revisit layout with #85453096 since original mockups didn't account for comments.

</tr>
<% end %>
</table>
<% end %>
Copy link
Contributor

Choose a reason for hiding this comment

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

Still feeling like it's not worth duplicating the markup for displaying cart details...

@afeld
Copy link
Contributor

afeld commented Feb 6, 2015

Mind posting a screenshot?

@phirefly
Copy link
Contributor Author

phirefly commented Feb 6, 2015

screen shot 2015-02-06 at 11 20 32 am

This could probably use a couple of alignment tweaks before merging this in.

<p class='comment-sender col-sm-6 col-xs-12'>
<strong>requester@test.com</strong>
</p>
<% unless c.user.nil? %>
Copy link
Contributor

Choose a reason for hiding this comment

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

is this "unless" actually needed?

@phirefly
Copy link
Contributor Author

phirefly commented Feb 6, 2015

Updated with some minor alignment adjustments

screen shot 2015-02-06 at 2 37 50 pm

@phirefly
Copy link
Contributor Author

phirefly commented Feb 6, 2015

Last bit of tweaks for now.

screen shot 2015-02-06 at 3 19 52 pm

afeld added a commit that referenced this pull request Feb 6, 2015
@afeld afeld merged commit b0c0dbb into master Feb 6, 2015
@afeld afeld deleted the 86661892_notifications_include_comments branch February 6, 2015 21:22
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