Skip to content
This repository was archived by the owner on Aug 1, 2022. It is now read-only.

refactor: improve project and source browsing#1080

Merged
xla merged 135 commits intomasterfrom
xla/refactor-peer-selection
Nov 19, 2020
Merged

refactor: improve project and source browsing#1080
xla merged 135 commits intomasterfrom
xla/refactor-peer-selection

Conversation

@xla
Copy link
Contributor

@xla xla commented Oct 20, 2020

Fixes #970
Fixes #1141
Fixes #1145

@xla xla added this to the Housekeeping milestone Oct 20, 2020
@xla xla self-assigned this Oct 20, 2020
FintanH and others added 5 commits October 20, 2020 17:12
We refactor tracked so that it returns the most information as possible.
There are three cases in particular:
* Searching -- peer is tracked but no information is available for them
yet
* Maintainer -- peer is a maintainer of the project
* Tracking -- peer is a contributor to the project
@xla xla changed the base branch from master to fintan/list-remotes October 21, 2020 08:04
@xla xla force-pushed the xla/refactor-peer-selection branch from 91f2963 to d7b2b7a Compare October 21, 2020 08:06
@xla xla force-pushed the xla/refactor-peer-selection branch from d7b2b7a to 2e1b232 Compare October 21, 2020 08:11
@xla xla force-pushed the xla/refactor-peer-selection branch from 62560a4 to 8b3def2 Compare October 21, 2020 10:26
@xla xla changed the title refactor: generalise peer selection refactor: improve project and source browsing Oct 23, 2020
Base automatically changed from fintan/list-remotes to master October 24, 2020 08:10
@juliendonck juliendonck linked an issue Oct 30, 2020 that may be closed by this pull request
@xla xla linked an issue Oct 30, 2020 that may be closed by this pull request
@rudolfs
Copy link
Member

rudolfs commented Nov 18, 2020

Revision selector looks wonky:

revision selector peer selector
dropdown peer
  • the expand icon should be visible in both expanded and collapsed states (like in peer selector)
  • the expand icon is misaligned

@xla
Copy link
Contributor Author

xla commented Nov 18, 2020

One note (not a blocker): Once I click on any file (commit is in the file) I would expect to always be able to click back on "Files" and that resets the source view, meaning that the latest commit would be back above the file. Now I can only get back there by refreshing the app.

This is implemented in 26d16bf

@rudolfs
Copy link
Member

rudolfs commented Nov 18, 2020

Revision selector ordering is mixed up, the selected branch is always shown first in the list:

master xla/refactor-peer-selection
master xla

@xla
Copy link
Contributor Author

xla commented Nov 18, 2020

the expand icon should be visible in both expanded and collapsed states (like in peer selector)

This is how master works as well, can change it. Just wanna make sure that we want all selectors work the same.

Revision selector ordering is mixed up, the selected branch is always shown first in the list:

This was suggested by @cloudhead and I agree that it makes sense with our current dropdown UX. We can review that once we make it more sophisticated: highlighting default branch, add filtering to the dropdown.

@xla xla requested review from cloudhead and juliendonck November 18, 2020 18:41
@cloudhead
Copy link
Contributor

Yeah it now behaves how dropdowns are supposed to behave 👍

@cloudhead
Copy link
Contributor

cloudhead commented Nov 18, 2020

the expand icon should be visible in both expanded and collapsed states (like in peer selector)

This seems strange, where do you see that?

In Figma you can see that the expand button isn't there when expanded, which makes sense: https://www.figma.com/file/owmgsbs6lnUt8R1bixstCA/%F0%9F%8C%B1%F0%9F%86%99-Radicle-Upstream?node-id=4147%3A7516

@cloudhead
Copy link
Contributor

cloudhead commented Nov 18, 2020

This is implemented in 26d16bf

Could we preserve the behavior though when you click on it from the Commits view? So it preserves the file you were on, but if you're already on the Files view, then it goes to root? 😬

@xla
Copy link
Contributor Author

xla commented Nov 19, 2020

Could we preserve the behavior though when you click on it from the Commits view? So it preserves the file you were on, but if you're already on the Files view, then it goes to root? grimacing

Done in 597cb35

@rudolfs
Copy link
Member

rudolfs commented Nov 19, 2020

This seems strange, where do you see that?

In Figma you can see that the expand button isn't there when expanded, which makes sense: https://www.figma.com/file/owmgsbs6lnUt8R1bixstCA/%F0%9F%8C%B1%F0%9F%86%99-Radicle-Upstream?node-id=4147%3A7516

@cloudhead you're right. We changed the peer selector here: #1261 (comment). Now that I look at it again, I think we should have just added a spacer that takes up as much space as the icon to avoid the "jumping", thanks for pointing out!

rudolfs
rudolfs previously approved these changes Nov 19, 2020
Copy link
Member

@rudolfs rudolfs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work! 🚀

Found just one small thing:

  • Figma reference 8px:
    Screenshot 2020-11-19 at 09 33 42

  • We have 12px:

Screenshot 2020-11-19 at 09 34 48

Not a blocker, can also be addressed later.

@xla
Copy link
Contributor Author

xla commented Nov 19, 2020

Not a blocker, can also be addressed later.

Was triggering me as well, do we have a proper rem value for it?

@rudolfs
Copy link
Member

rudolfs commented Nov 19, 2020

Was triggering me as well, do we have a proper rem value for it?

@xla 0.5rem

Copy link
Contributor

@MeBrei MeBrei left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⭐ ⛰️ ⭐

@juliendonck
Copy link
Member

@xla merge it! quick!
Screen Shot 2020-11-19 at 11 34 41

Copy link
Contributor

@cloudhead cloudhead left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎂

@xla xla merged commit 892233d into master Nov 19, 2020
@xla xla deleted the xla/refactor-peer-selection branch November 19, 2020 11:10
xla added a commit that referenced this pull request Nov 20, 2020
Since #1080 constrainted the peer list correctly we had to fix the
incorrect role assigned. Can't think of a case where the remote peer can
be of type `Tracker` at this point.

Fixes #1297
xla added a commit that referenced this pull request Nov 20, 2020
Since #1080 constrainted the peer list correctly we had to fix the
incorrect role assigned. Can't think of a case where the remote peer can
be of type `Tracker` at this point.

Fixes #1297
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Switching between repos when file is selected breaks UI "Incorrect input" when opening project Project readme flickers on branch switch

7 participants