Uploaded image for project: 'phpBB3'
  1. phpBB3
  2. PHPBB3-10050

subsilver2: Do not show "Mark topics as read" when there are no topics

    Details

    • Type: Improvement
    • Status: Unverified Fix
    • Priority: Minor
    • Resolution: Fixed
    • Affects Version/s: 3.0.8
    • Fix Version/s: 3.0.12-RC1
    • Component/s: Viewing forums
    • Labels:
      None

      Description

      in the an empty forum displays the "Mark topics as read", even though there is no topics

      can see in attached picture.

      Regards,

      Logman

        Activity

        Hide
        Oleg Oleg [X] (Inactive) added a comment - - edited

        If the most recent topic in a forum is new and is also older than the time cutoff, there may be new topics in the forum while no topics are displayed.

        Show
        Oleg Oleg [X] (Inactive) added a comment - - edited If the most recent topic in a forum is new and is also older than the time cutoff, there may be new topics in the forum while no topics are displayed.
        Hide
        Ger Ger added a comment -

        @Oleg: even then there should not be a link to mark topics read while there aren't any topics visible. It just isn't clear for the user what he's marking as read.

        Proposed patch:
        OPEN ./styles/prosilver/template/viewforum_body.html

        FIND

        <!-- IF not S_IS_BOT and U_MARK_TOPICS --><a href="{U_MARK_TOPICS}" accesskey="m">{L_MARK_TOPICS_READ}</a> &bull; <!-- ENDIF --><!-- IF TOTAL_TOPICS -->{TOTAL_TOPICS}<!-- ENDIF -->

        REPLACE WITH

        <!-- IF not S_IS_BOT and U_MARK_TOPICS and TOTAL_TOPICS != 0 --><a href="{U_MARK_TOPICS}" accesskey="m">{L_MARK_TOPICS_READ}</a> &bull; <!-- ENDIF --><!-- IF TOTAL_TOPICS -->{TOTAL_TOPICS}<!-- ENDIF -->

        OPEN
        ./style/subsilver2/template/viewforum_body.html

        FIND

        <td align="{S_CONTENT_FLOW_END}" valign="middle"><!-- IF not S_IS_BOT and U_MARK_TOPICS and TOTAL_TOPICS != 0 --><a href="{U_MARK_TOPICS}">{L_MARK_TOPICS_READ}</a><!-- ENDIF -->&nbsp;</td>

        REPLACE WITH

        <td align="{S_CONTENT_FLOW_END}" valign="middle"><!-- IF not S_IS_BOT and U_MARK_TOPICS --><a href="{U_MARK_TOPICS}">{L_MARK_TOPICS_READ}</a><!-- ENDIF -->&nbsp;</td>

        Show
        Ger Ger added a comment - @Oleg: even then there should not be a link to mark topics read while there aren't any topics visible. It just isn't clear for the user what he's marking as read. Proposed patch: OPEN ./styles/prosilver/template/viewforum_body.html FIND <!-- IF not S_IS_BOT and U_MARK_TOPICS --><a href="{U_MARK_TOPICS}" accesskey="m">{L_MARK_TOPICS_READ}</a> &bull; <!-- ENDIF --><!-- IF TOTAL_TOPICS -->{TOTAL_TOPICS}<!-- ENDIF --> REPLACE WITH <!-- IF not S_IS_BOT and U_MARK_TOPICS and TOTAL_TOPICS != 0 --><a href="{U_MARK_TOPICS}" accesskey="m">{L_MARK_TOPICS_READ}</a> &bull; <!-- ENDIF --><!-- IF TOTAL_TOPICS -->{TOTAL_TOPICS}<!-- ENDIF --> OPEN ./style/subsilver2/template/viewforum_body.html FIND <td align="{S_CONTENT_FLOW_END}" valign="middle"><!-- IF not S_IS_BOT and U_MARK_TOPICS and TOTAL_TOPICS != 0 --><a href="{U_MARK_TOPICS}">{L_MARK_TOPICS_READ}</a><!-- ENDIF -->&nbsp;</td> REPLACE WITH <td align="{S_CONTENT_FLOW_END}" valign="middle"><!-- IF not S_IS_BOT and U_MARK_TOPICS --><a href="{U_MARK_TOPICS}">{L_MARK_TOPICS_READ}</a><!-- ENDIF -->&nbsp;</td>
        Hide
        Oleg Oleg [X] (Inactive) added a comment - - edited

        Testing https://github.com/phpbb/phpbb3/pull/919:

        1. QI-install a populated board. You get some number of topics per forum which are all unread.

        2. Go into the forum and create another page (25+) worth of topics. These are read since you created them.

        3. Go to the forum (topic listing).

        With the patch applied, "mark topics read" is gone. However there are unread topics on page 2.

        It may be the case that "mark topics read" is not quite relevant. It is more accurately "mark forum read". If this change is made, then hiding this link should only be done when all topics in the forum are read, and normal viewforum code in my estimation never fetches all topics for display purposes due to pagination/other constraints, therefore either you need to find a query that does pull something out of the entire set of topics in the forum and piggy-back onto it, or a patch would need to add a query to determine whether or not there are any topics that are unread.

        Correctness check: the link should appear if and only if the forum icon on forum listing shows unread topics exist in the forum.

        Show
        Oleg Oleg [X] (Inactive) added a comment - - edited Testing https://github.com/phpbb/phpbb3/pull/919: 1. QI-install a populated board. You get some number of topics per forum which are all unread. 2. Go into the forum and create another page (25+) worth of topics. These are read since you created them. 3. Go to the forum (topic listing). With the patch applied, "mark topics read" is gone. However there are unread topics on page 2. It may be the case that "mark topics read" is not quite relevant. It is more accurately "mark forum read". If this change is made, then hiding this link should only be done when all topics in the forum are read, and normal viewforum code in my estimation never fetches all topics for display purposes due to pagination/other constraints, therefore either you need to find a query that does pull something out of the entire set of topics in the forum and piggy-back onto it, or a patch would need to add a query to determine whether or not there are any topics that are unread. Correctness check: the link should appear if and only if the forum icon on forum listing shows unread topics exist in the forum.
        Hide
        Senky Senky added a comment -

        After further inspection in PR, we realised that there are some special cases, when we are able to say for sure there are no unread topics (when topics count is the same as count of rows returned by data query), or there is an option to slightly update one query to get needed data, but only in case forum does not have subforums.
        So there are 2 specific cases, when we can hide the link for sure, but I think it is rather evil to hide them only for that cases. Your opinions?

        We are able to cover all cases only adding additional query, which is, for me and for p, too, not worth of it. Again, your opinions?

        Show
        Senky Senky added a comment - After further inspection in PR, we realised that there are some special cases, when we are able to say for sure there are no unread topics (when topics count is the same as count of rows returned by data query), or there is an option to slightly update one query to get needed data, but only in case forum does not have subforums. So there are 2 specific cases, when we can hide the link for sure, but I think it is rather evil to hide them only for that cases. Your opinions? We are able to cover all cases only adding additional query, which is, for me and for p, too, not worth of it. Again, your opinions?
        Hide
        EXreaction EXreaction [X] (Inactive) added a comment - - edited

        The PR has been merged, however, as per the comments on github, this only corrected behavior on subsilver2.
        https://github.com/phpbb/phpbb3/pull/919#issuecomment-12567979

        A subtask was created to handle prosilver specifically, once it is completed this can be marked as fixed.

        Show
        EXreaction EXreaction [X] (Inactive) added a comment - - edited The PR has been merged, however, as per the comments on github, this only corrected behavior on subsilver2. https://github.com/phpbb/phpbb3/pull/919#issuecomment-12567979 A subtask was created to handle prosilver specifically, once it is completed this can be marked as fixed.
        Hide
        bantu Andreas Fischer added a comment -

        Not sure why this got merged if it is incomplete. Upgrading to blocker then.

        Show
        bantu Andreas Fischer added a comment - Not sure why this got merged if it is incomplete. Upgrading to blocker then.
        Hide
        bantu Andreas Fischer added a comment -

        Closing this ticket for 3.0.12-RC1. Converted sub-task into issue for 3.0.13-RC1. http://tracker.phpbb.com/browse/PHPBB3-11346

        Show
        bantu Andreas Fischer added a comment - Closing this ticket for 3.0.12-RC1. Converted sub-task into issue for 3.0.13-RC1. http://tracker.phpbb.com/browse/PHPBB3-11346

          People

          • Assignee:
            EXreaction EXreaction [X] (Inactive)
            Reporter:
            Logman Logman
          • Votes:
            0 Vote for this issue
            Watchers:
            2 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development