Skip to content
This repository has been archived by the owner on Dec 15, 2022. It is now read-only.

Multiple tabs within an IssueishPaneItem #1656

Closed
smashwilson opened this issue Aug 20, 2018 · 8 comments
Closed

Multiple tabs within an IssueishPaneItem #1656

smashwilson opened this issue Aug 20, 2018 · 8 comments

Comments

@smashwilson
Copy link
Contributor

To pave the way for pull request review display, let's split the IssueishPaneItem's timeline into a series of tabs, each containing a subview of the pull request:

  • Description
  • Timeline (the existing view, with non-review comments, push events, and so on)
  • Commits
  • Changes: the full diff of the pull request, with files navigable as in Commit pane item #1655

We can render them horizontally in the workspace center and accordion-style when docked.

@simurai
Copy link
Contributor

simurai commented Aug 29, 2018

image

Should the CI checks be under description? Also toying with the idea of reverse the order:

image

Like in the context of an editor, the changes get the biggest focus.

@kuychaco
Copy link
Contributor

kuychaco commented Sep 6, 2018

Exciting to see the PR features taking shape!

I'm curious about the reasoning behind separating "Description" and "Timeline" as opposed to showing them in the same tab like we do on dotcom (under "Conversation"):

webgl_log_view_research_spike_by_smashwilson_ _pull_request__1678_ _atom_github

I'm all for deviating from dotcom where it makes sense, like @simurai is suggesting with reversing the order. Just want to understand the logic behind the deviation.

Personally, I like the "Conversation" tab we have on the website with both the description and then any timeline events because it does feel like a conversation to me. I'm not super attached to this though.

Reversing the order is an interesting suggestion. I'm not sure which I prefer at this point. This is definitely something I'd like to get some UXR data on.

@smashwilson
Copy link
Contributor Author

I'm curious about the reasoning behind separating "Description" and "Timeline" as opposed to showing them in the same tab

In my original write-up? Totally arbitrary, haha. I think it was mostly to have a nice even four items.

Do we just want to take the conversation / commits / checks / files changed headers from dotcom, and then add in reviews later? It might be jarring to start synced up and then immediately deviate. But I do kind of like that breakdown, and it's be nice to have that instant familiarity with what users would expect to see where.

Reversing the order is an interesting suggestion. I'm not sure which I prefer at this point. This is definitely something I'd like to get some UXR data on.

Me neither! I kind of default to wanting to start consistent in order as well?

@annthurium annthurium self-assigned this Sep 7, 2018
@annthurium
Copy link
Contributor

ok so another question...if we go for Conversations, Commits, Checks, Files Changed, are we going to support the Checks api since that's what lives on that tab in dotcom? Or do we just stuff the CI status checks that already exist in GitHub package on that tab?

@simurai
Copy link
Contributor

simurai commented Sep 10, 2018

it's be nice to have that instant familiarity with what users would expect to see where.

Yeah, the familiarity is a big plus. At the same time it could lead to confusion and frustration. If people see the same navigation as on .com, they might expect the same functionality when clicking the tabs. People will be like "Ohh.. right. I can't do this here" and the more this happens, the less they trust these tabs. If we copy .com, we kinda have to copy 100% of it. Like putting an iframe in the center pane. So by not doing the same as on .com, we can better manage expectations and it doesn't feel strange if some parts are missing or are different.

This question about how close should it be to .com comes up from time to time, and we haven't really found a clear answer. Lately I started to lean more towards not trying to make it close to .com. Mainly because of our team size, it seems impossible to ever catch up. But also if we don't copy .com 1:1, it leaves some room to do things that are not possible on .com and only make sense in the context of an editor. I know, much easier said than done. 😅

@smashwilson
Copy link
Contributor Author

If people see the same navigation as on .com, they might expect the same functionality when clicking the tabs. People will be like "Ohh.. right. I can't do this here" and the more this happens, the less they trust these tabs. If we copy .com, we kinda have to copy 100% of it. Like putting an iframe in the center pane. So by not doing the same as on .com, we can better manage expectations and it doesn't feel strange if some parts are missing or are different.

Yeah, that's a really good point. It already feels low-key not-great that you can't comment from the Atom pane item, or add labels, etc.

ok so another question...if we go for Conversations, Commits, Checks, Files Changed, are we going to support the Checks api since that's what lives on that tab in dotcom? Or do we just stuff the CI status checks that already exist in GitHub package on that tab?

Oh, yeah, hmm.

It would be awesome to have proper checks support eventually! Being able to jump directly to the line where a linter failed (say) would be really handy. But you're right that that would be a bigger chunk of work to support. Maybe we could go with putting the existing status summary in a Status tab and open a new issue to enhance it with checks?

@annthurium
Copy link
Contributor

okay, so:

  • @kuychaco feels strongly that separating "Description" and "Timeline" would lead to a lot of clicking back and forth. That seems reasonable. I don't think we should call it "Conversation" though because that's too close to what dotcom uses, and it's only a conversation when you can actually comment. So let's call that tab "Overview."

  • I think "Build Status" can go on its own tab.

  • It doesn't make that much sense to have a "Commits" tab until we have a nice view of a single commit.

Which leaves our tab list as:

  • Overview
  • Build Status
  • Files Changed

With these tabs to be added in future PRs:

  • Commits
  • Reviews

@simurai
Copy link
Contributor

simurai commented Sep 11, 2018

So let's call that tab "Overview."

I like "Overview". 👍 👍 👍 Then we can change the content in the future without having to change the label. It will always stay the "overview of a PR".

We could also consider keeping the build status in the Overview. Since it's something people might want to check often and wouldn't need to do an extra click. It also doesn't take up THAT much height. Then the tabs would initially only be Overview | Files Changed, which looks odd, but maybe ok as a stop-gap.

Feature Sprint : 27 August - 14 September 2018 : v0.20.0 automation moved this from In Progress 🔧 to Merged ☑️ Sep 25, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
No open projects
Development

No branches or pull requests

4 participants