This is a lighter UI treatment for previous/forward comments, where the comment is just hidden behind a click. This mayyy be too hard to discover, but I'd rather wait and make it more obvious if actually needed. Once you understand the interaction, the cleaner UI is preferable. Ref T7447
Details
- Reviewers
epriestley btrahan - Maniphest Tasks
- T7447: Bring inline comments forward across revision updates
- Commits
- Restricted Diffusion Commit
rP68c113832e82: Collapse previous/forward by default, click to expand.
Test a lot of previous and forward comments.
Diff Detail
- Repository
- rP Phabricator
- Lint
Lint Not Applicable - Unit
Tests Not Applicable
Event Timeline
src/infrastructure/diff/view/PHUIDiffInlineCommentDetailView.php | ||
---|---|---|
427 | whoopsies |
I'm personally quite happy with things as they are, but this is easier than some of the other suggestions, so let's see how it works.
After using these, I've literally clicked every single one of these I've hit. I never remember what they said and always want to read them.
Maybe we should add a preference?
- Show Full
- Show Collapsed (Default)
- Hide
I hate adding preferences, but this feature was like 10x better for me before the collapse change.
I can try to get summarizing kind of working before we go that far, I just think it's going to be junk in a bunch of cases. Not sure if that will be 2% of cases or 35%.
I'd remove the collapse and just make a JS toggle / pref. show old / hide old. I don't mind preferences when I feel people are going to be opinionated on both sides and we want everyone to feel productive.
Probably still keep some icon indication ghost comments exist though even if you have them off?
What can I do here for next steps then?
- Remove 'minimal UI'
- Add keyboard shortcut to toggle ghost on and off
- Add sticky preference (or settings preference?)
- Add UI hint that old comments exist
I can do some of these, just didn't know if you were also working on them.
Let me try to do summaries first and see how bad that actually is, I really hate preferences.
If that route, probably also open them all with one click instead of individual. Like, you either want to read it all or just have a small summary.
Yeah I faked it by just pulling up the text from the first p tag, and hiding everything else.