Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

Add link to all commits from tree page#60909

Merged
gabtorre merged 8 commits into
mainfrom
gabe/add-commits-link
Mar 14, 2024
Merged

Add link to all commits from tree page#60909
gabtorre merged 8 commits into
mainfrom
gabe/add-commits-link

Conversation

@gabtorre

@gabtorre gabtorre commented Mar 7, 2024

Copy link
Copy Markdown
Contributor

Closes https://github.com/sourcegraph/sourcegraph/issues/60867

This PR adds a link to the page showing all commits like the repo root page.

Test plan

Manually tested

Screenshot 2024-03-13 at 6 38 27 PM

@cla-bot cla-bot Bot added the cla-signed label Mar 7, 2024
@jdm-twtr

jdm-twtr commented Mar 7, 2024

Copy link
Copy Markdown

I think this is a reasonable stop-gap to the issue! Putting my UX hat on now :) the placement of "show all commits" does not feel like it belongs in the directory table. I'd imagine a standalone button centered below the file list, in place of where the previous commit history table was. Users familiar with Sourcegraph will be looking there for the commit history, and find the button instead, which is less drastic of a change than placing a text link in an adjacent, arguably unrelated table.

Just my 2 cents.

@gabtorre gabtorre requested a review from fkling March 7, 2024 05:02
@fkling

fkling commented Mar 11, 2024

Copy link
Copy Markdown
Contributor

Sorry, this slipped through the cracks... there seem to be a lot of unrelated changes in this PR, can you rebase?

My gut reaction is that the link feels a bit out of place in this view. Although I'm not a big fan of this either, what do you think about adding a link/button similar to one the one the repo homepage:

2024-03-11_21-20

That would at least make things consistent.

@gabtorre gabtorre force-pushed the gabe/add-commits-link branch from c6a2b18 to ea1fbf4 Compare March 11, 2024 23:55
@gabtorre

Copy link
Copy Markdown
Contributor Author

That makes sense. Would something like this work better? @fkling

Screenshot 2024-03-11 at 4 06 39 PM

@fkling

fkling commented Mar 13, 2024

Copy link
Copy Markdown
Contributor

@gabtorre Yes!

@peterguy

peterguy commented Mar 14, 2024

Copy link
Copy Markdown
Contributor

@gabtorre I don't see any Commits button when running it locally. Maybe I did something wrong; you can audit my steps to make sure:

I also don't see any ownership info; not sure if that's an indicator of something broken on my end or not.

I do see the Show all commits link (which works fine), so perhaps I'm commenting too soon?

screen capture of the folder view

@gabtorre

Copy link
Copy Markdown
Contributor Author

I just pushed the changes and updated the description, sorry about that! @peterguy

Comment thread client/web/src/repo/tree/TreePageContent.tsx Outdated
Comment thread client/web/src/repo/tree/TreePageContent.tsx
@gabtorre gabtorre requested a review from peterguy March 14, 2024 03:36
@peterguy

Copy link
Copy Markdown
Contributor

Looks great, thanks, @gabtorre!

@peterguy peterguy left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good!

Comment thread client/web/src/repo/tree/TreePageContent.tsx Outdated
@gabtorre gabtorre requested a review from fkling March 14, 2024 17:32
@peterguy

Copy link
Copy Markdown
Contributor

@gabtorre I double-checked your modification to include the revision in the link to the commits. Works well for branches, tags, and commits. Looks good!

@gabtorre gabtorre merged commit c326671 into main Mar 14, 2024
@gabtorre gabtorre deleted the gabe/add-commits-link branch March 14, 2024 21:09
peterguy added a commit that referenced this pull request Mar 27, 2024
…page.

Followup to PR #60909, which added a Commits button to the folders,
and included the revision in it, expoing the shortcome of the Commits button
in the repo root page, which didn't include the revision.
peterguy added a commit that referenced this pull request Mar 28, 2024
…ology (#61408)

* Add the current revision to the Commits button link on the repo root page.

Followup to PR #60909, which added a Commits button to the folders,
and included the revision in it, expoing the shortcome of the Commits button
in the repo root page, which didn't include the revision.

* Refactor commin code into a function

And add handling for Perforce depots, so that the
button shows "changelists" for Perforce instead
of "commits" and it navigates to `/-/changelists`
instead of to `/-/commits`.

* Remove unnecessary fragment wrapper tags

* Use inline `RepoCommitsButton` instead of treating it as a function

* Add tests for TreePage and the new Commits button language

* test using toBeInTheDocument()

* Add a name attribute to the button for testing.
And move the `as` attribute to the top to make it clear that it renders as a link.

* Add tests for the button link to make sure that it builds the url correctly.

* Add CHANGELOG entries for the Commits button changes
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Directory Commits Page Not Accessible

4 participants