Skip to content

Commit

Permalink
add option suppress_reply_directly_above to stop suppressing the repl…
Browse files Browse the repository at this point in the history
…y directly above

added a bunch of debugging information to help diagnose weird positioning issues
  • Loading branch information
SamSaffron committed Jul 8, 2013
1 parent c6c8246 commit d79aa91
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 18 deletions.
5 changes: 4 additions & 1 deletion app/assets/javascripts/discourse/models/post.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,10 @@ Discourse.Post = Discourse.Model.extend({
}.property('username'),

showUserReplyTab: function() {
return this.get('reply_to_user') && (this.get('reply_to_post_number') < (this.get('post_number') - 1));
return this.get('reply_to_user') && (
!Discourse.SiteSettings.suppress_reply_directly_above ||
this.get('reply_to_post_number') < (this.get('post_number') - 1)
);
}.property('reply_to_user', 'reply_to_post_number', 'post_number'),

byTopicCreator: function() {
Expand Down
16 changes: 12 additions & 4 deletions app/assets/javascripts/discourse/views/post_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,16 @@ Discourse.PostView = Discourse.View.extend({
// Toggle visibility of parent post
toggleParent: function(e) {
var postView = this;
var post = this.get('post');
var $parent = this.$('.parent-post');
var inReplyTo = post.get('reply_to_post_number');

if (post.get('post_number') - 1 === inReplyTo) {
// true means ... avoid scroll if possible
Discourse.TopicView.jumpToPost(post.get('topic_id'), inReplyTo, true);
return;
}

if (this.get('parentPost')) {
$('nav', $parent).removeClass('toggled');
// Don't animate on touch
Expand All @@ -80,11 +89,10 @@ Discourse.PostView = Discourse.View.extend({
$parent.slideUp(function() { postView.set('parentPost', null); });
}
} else {
var post = this.get('post');
this.set('loadingParent', true);
$('nav', $parent).addClass('toggled');

Discourse.Post.loadByPostNumber(post.get('topic_id'), post.get('reply_to_post_number')).then(function(result) {
Discourse.Post.loadByPostNumber(post.get('topic_id'), inReplyTo).then(function(result) {
postView.set('loadingParent', false);
// Give the post a reference back to the topic
result.topic = postView.get('post.topic');
Expand Down Expand Up @@ -159,9 +167,9 @@ Discourse.PostView = Discourse.View.extend({
showLinkCounts: function() {

var postView = this;
var link_counts;
var link_counts = this.get('post.link_counts');

if (link_counts = this.get('post.link_counts')) {
if (link_counts) {
_.each(link_counts, function(lc) {
if (lc.clicks > 0) {
postView.$(".cooked a[href]").each(function() {
Expand Down
78 changes: 65 additions & 13 deletions app/assets/javascripts/discourse/views/topic_view.js
Original file line number Diff line number Diff line change
Expand Up @@ -329,30 +329,82 @@ Discourse.TopicView = Discourse.View.extend(Discourse.Scrolling, {
Discourse.TopicView.reopenClass({

// Scroll to a given post, if in the DOM. Returns whether it was in the DOM or not.
jumpToPost: function(topicId, postNumber) {
jumpToPost: function(topicId, postNumber, avoidScrollIfPossible) {
Em.run.scheduleOnce('afterRender', function() {

var rows = $('.topic-post.ready');

// Make sure we're looking at the topic we want to scroll to
if (topicId !== parseInt($('#topic').data('topic-id'), 10)) { return false; }

var $post = $("#post_" + postNumber);
if ($post.length) {
if (postNumber === 1) {
$('html, body').scrollTop(0);
} else {
var header = $('header');
var title = $('#topic-title');
var expectedOffset = title.height() - header.find('.contents').height();

if (expectedOffset < 0) {
expectedOffset = 0;
}
var postTop = $post.offset().top;
var highlight = true;

var header = $('header');
var title = $('#topic-title');
var expectedOffset = title.height() - header.find('.contents').height();
console.log(expectedOffset);

if (expectedOffset < 0) {
expectedOffset = 0;
}

$('html, body').scrollTop($post.offset().top - (header.outerHeight(true) + expectedOffset));
var offset = (header.outerHeight(true) + expectedOffset);
var windowScrollTop = $('html, body').scrollTop();

if (avoidScrollIfPossible && postTop > windowScrollTop + offset && postTop < windowScrollTop + $(window).height() + 100) {
// in view
} else {
// not in view ... bring into view
if (postNumber === 1) {
$(window).scrollTop(0);
highlight = false;
} else {
var desired = $post.offset().top - offset;
$(window).scrollTop(desired);

// TODO @Robin, I am seeing multiple events in chrome issued after
// jumpToPost if I refresh a page, sometimes I see 2, sometimes 3
//
// 1. Where are they coming from?
// 2. On refresh we should only issue a single scrollTop
// 3. If you are scrolled down in BoingBoing desired sometimes is wrong
// due to vanishing header, we should not be rendering it imho until after
// we render the posts

var first = true;
var t = new Date();
console.log("DESIRED:" + desired);
var enforceDesired = function(){
if($(window).scrollTop() !== desired) {
console.log("GOT EVENT " + $(window).scrollTop());
console.log("Time " + (new Date() - t));
console.trace();
if(first) {
$(window).scrollTop(desired);
first = false;
}
// $(document).unbind("scroll", enforceDesired);
}
};

// uncomment this line to help debug this issue.
// $(document).scroll(enforceDesired);
}
}

if(highlight) {
var $contents = $('.topic-body .contents', $post);
var originalCol = $contents.css('backgroundColor');
$contents.css({ backgroundColor: "#ffffcc" }).animate({ backgroundColor: originalCol }, 2500);
var origColor = $contents.data('orig-color') || $contents.css('backgroundColor');

$contents.data("orig-color", origColor);
$contents
.css({ backgroundColor: "#ffffcc" })
.stop()
.animate({ backgroundColor: origColor }, 2500);
}
}
});
Expand Down
1 change: 1 addition & 0 deletions app/models/site_setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ class SiteSetting < ActiveRecord::Base
client_setting(:min_search_term_length, 3)
client_setting(:flush_timings_secs, 5)
client_setting(:suppress_reply_directly_below, true)
client_setting(:suppress_reply_directly_above, true)
client_setting(:email_domains_blacklist, 'mailinator.com')
client_setting(:email_domains_whitelist)
client_setting(:version_checks, true)
Expand Down
2 changes: 2 additions & 0 deletions config/locales/server.en.yml
Original file line number Diff line number Diff line change
Expand Up @@ -514,6 +514,8 @@ en:
system_username: "Username for the author of automated private messages sent by the forum"
send_welcome_message: "Do new users get a welcome private message?"
suppress_reply_directly_below: "Don't show reply count on a post when there is a single reply directly below"
suppress_reply_directly_above: "Don't show in-reply-to on a post when there is a single reply directly above"

allow_index_in_robots_txt: "Site should be indexed by search engines (update robots.txt)"
email_domains_blacklist: "A pipe-delimited list of email domains that are not allowed. Example: mailinator.com|trashmail.net"
email_domains_whitelist: "A pipe-delimited list of email domains that users may register with. WARNING: Users with email domains other than those listed will not be allowed."
Expand Down

0 comments on commit d79aa91

Please sign in to comment.