Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#1113] Show the read comments link when you reply via QR #1270

Merged
merged 1 commit into from
Mar 6, 2015

Conversation

afuna
Copy link
Member

@afuna afuna commented Mar 3, 2015

-- this is for the case where we go from 0 -> 1 comment, and we used to
not display a link (because we had nothing to display)

Fixes #1113.

@afuna
Copy link
Member Author

afuna commented Mar 3, 2015

Hmm, might need to think about this a bit more. This means that layouts that aren't based on core2 will suddenly have "0 comments" all over their read page.

@afuna
Copy link
Member Author

afuna commented Mar 3, 2015

pls to feedback if able, but don't merge yet. thanks!

@zorkian
Copy link
Member

zorkian commented Mar 4, 2015

Ah so the goal here is that when you QR on your reading page, you want the comments link to appear? That sounds good, but I definitely agree that we shouldn't show the "0 comments" text to everybody -- that seems funky.

@zorkian
Copy link
Member

zorkian commented Mar 4, 2015

PS marking as changes requested to block merging.

-- this is for the case where we go from 0 -> 1 comment, and we used to
not display a link (because we had nothing to display)

Fixes dreamwidth#1113.
@afuna
Copy link
Member Author

afuna commented Mar 4, 2015

Figured out how to make this work for core2 styles without affecting custom styles that haven't been updated: use a different variable. Removing label as this one is suitable for review and/or merge.

@afuna
Copy link
Member Author

afuna commented Mar 4, 2015

Hey also -- do we care about labelling pull requests? I mostly focus on labelling the issues and haven't paid attention to the associated pull requests.

@zorkian
Copy link
Member

zorkian commented Mar 4, 2015

What do you mean labeling?

Mark Smith mark@qq.is

On Wed, Mar 4, 2015, at 10:19 AM, Afuna wrote:

Hey also -- do we care about labelling pull requests? I mostly focus
on labelling the issues and haven't paid attention to the associated
pull requests.

— Reply to this email directly or view it on GitHub[1].

Links:

  1. [#1113] Show the read comments link when you reply via QR #1270 (comment)

@afuna
Copy link
Member Author

afuna commented Mar 4, 2015

Adding the severity, etc.

On Wed, Mar 4, 2015, at 11:50 AM, Mark Smith wrote:

What do you mean labeling?

Mark Smith mark@qq.is

On Wed, Mar 4, 2015, at 10:19 AM, Afuna wrote:

Hey also -- do we care about labelling pull requests? I mostly focus

on labelling the issues and haven't paid attention to the associated

pull requests.

— Reply to this email directly or view it on GitHub[1].

Links:

  1. [#1113] Show the read comments link when you reply via QR #1270 (comment)
    — Reply to this email directly or view it on GitHub[1].

Links:

  1. [#1113] Show the read comments link when you reply via QR #1270 (comment)

@zorkian
Copy link
Member

zorkian commented Mar 4, 2015

Since I only am looking at Pull Requests, I think it's useful to add
them?

Mark Smith mark@qq.is

On Wed, Mar 4, 2015, at 12:07 PM, Afuna wrote:

Adding the severity, etc.

On Wed, Mar 4, 2015, at 11:50 AM, Mark Smith wrote:

What do you mean labeling?

Mark Smith mark@qq.is

On Wed, Mar 4, 2015, at 10:19 AM, Afuna wrote:

Hey also -- do we care about labelling pull requests? I mostly focus

on labelling the issues and haven't paid attention to the associated

pull requests.

— Reply to this email directly or view it on GitHub[1].

Links:

  1. [#1113] Show the read comments link when you reply via QR #1270 (comment)

— Reply to this email directly or view it on GitHub[1].

Links:

  1. [#1113] Show the read comments link when you reply via QR #1270 (comment)
    — Reply to this email directly or view it on GitHub[1].

Links:

  1. [#1113] Show the read comments link when you reply via QR #1270 (comment)

@afuna
Copy link
Member Author

afuna commented Mar 4, 2015

Okay!

@zorkian
Copy link
Member

zorkian commented Mar 6, 2015

This introduces a new option? Hrmm, why?

You mention backwards compatibility with old styles. Is there really no way to do it with CSS/JS that makes it work for them without forcing people to update their style to turn this variable on?

@zorkian
Copy link
Member

zorkian commented Mar 6, 2015

Oh nevermind, it's entirely internal; when they recompile they'll get it.

zorkian added a commit that referenced this pull request Mar 6, 2015
[#1113] Show the read comments link when you reply via QR
@zorkian zorkian merged commit 8cb1ce1 into dreamwidth:develop Mar 6, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

make sure to show "1 comment" when we post inline from reading page to an entry which had 0 comments
3 participants