Page MenuHomePhabricator
Create Task
Maniphest T8420

Hidden inlines don't work properly with anchor-based navigation
Closed, ResolvedPublic
Actions

Description

If an inline comment is hidden as per D13009:

  • Clicking that comment's line number in the summary table no longer takes me to that portion of the diff
  • The "previous" / "next" buttons don't work on inline comments if the previous / next comment is hidden, even when visible comments exist beyond that one
  • Other anchor-based links to it don't work.

Revisions and Commits

rP Phabricator
D17910rP86b9deb8a9e9 Move inline anchors up, to dolumn-level

Related Objects

Event Timeline

staticshock created this task.Jun 4 2015, 3:51 PM
staticshock raised the priority of this task from to Needs Triage.
staticshock updated the task description. (Show Details)
staticshock added a project: Differential.
staticshock added a subscriber: staticshock.
joshuaspence added a subscriber: joshuaspence.Jun 4 2015, 8:34 PM
staticshock renamed this task from Line number in inline summary table doesn't take you to the line if the comment is hidden to Hiding comments breaks inline comment navigation.Jun 9 2015, 6:06 PM
staticshock updated the task description. (Show Details)
epriestley renamed this task from Hiding comments breaks inline comment navigation to Hidden inlines don't work properly with anchor-based navigation.Jun 20 2015, 12:14 PM
epriestley merged a task: T8622: Links to inline comments on older diffs are not working.
epriestley updated the task description. (Show Details)
epriestley added a subscriber: epriestley.

I didn't really implement anchor integration properly because I wasn't sure that hiding was going to stick, but I think hiding is pretty good.

epriestley mentioned this in T7447: Bring inline comments forward across revision updates.Jun 21 2015, 2:01 PM
chad merged a task: T9107: diffusion can't scroll past hidden comments.Aug 7 2015, 6:44 PM
chad added subscribers: chad, • sshannin.
Herald added a subscriber: eadler. · View Herald TranscriptAug 7 2015, 6:44 PM
chad merged a task: T9160: Hiding inline comments confuses keyboard navigation shortcuts.Aug 13 2015, 4:06 AM
chad added a subscriber: jparise.
joe.roback added a subscriber: joe.roback.Sep 28 2015, 10:33 PM
Herald added a subscriber: cspeckmim. · View Herald TranscriptSep 28 2015, 10:33 PM
joe.roback added a comment.Sep 28 2015, 10:34 PM

Hiding is completely essential. If you have 10 lines of code with 5 comments, where there are say 1 reply each, All the comments inline make it impossible to read the code anymore. Inline comments are not helpful for larger more complex code where the reviewer needs to reason about more than 1 line of code.

chad merged a task: T9503: previous/next buttons on inline comments should skip hidden comments.Oct 2 2015, 4:09 PM
chad added a subscriber: danielberger.
chad merged a task: T10230: Cannot jump to hidden inline comment.Jan 27 2016, 9:18 PM
epriestley mentioned this in T8909: Provide ability to hide all inline or done comments.Jun 17 2016, 4:23 PM
isfs added a subscriber: isfs.Jun 18 2016, 1:34 AM
eadler added a project: Restricted Project.Jul 7 2016, 5:14 PM
epriestley moved this task from Backlog to Inline Infra on the Differential board.Dec 29 2016, 4:27 PM
chris.siden added a subscriber: chris.siden.Jan 6 2017, 7:30 PM
kaveh added a subscriber: kaveh.Mar 22 2017, 9:15 PM
epriestley added a project: Inline Comments.Apr 21 2017, 12:20 PM
epriestley moved this task from Backlog to Glitches on the Inline Comments board.Apr 21 2017, 12:24 PM
epriestley added a revision: D17910: Move inline anchors up, to dolumn-level.May 16 2017, 5:08 PM
epriestley closed this task as Resolved by committing rP86b9deb8a9e9: Move inline anchors up, to dolumn-level.May 16 2017, 5:12 PM
epriestley added a commit: rP86b9deb8a9e9: Move inline anchors up, to dolumn-level.
Phabricator · Built by Phacility