Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

teams: new View button #27685

Merged
merged 4 commits into from
Oct 22, 2023
Merged

Conversation

tomholford
Copy link
Contributor

@tomholford tomholford commented Oct 19, 2023

Per the discussion on #22054, the flow for adding a new team member to an org is not intuitive for new Gitea users.

The ideal solution would be to add a new button on the Org > Members index view (see the screenshot mockup in the issue description). However, this would require a refactor of the UX for the flow. The current flow has an implicit context of which team within the org the new member is being added to ('Owners' by default). From the Members index, there is no implicit context; the flow would have to add a picker for which team the new member should be added to.

So, as a stopgap, this change simply adds a button to the Teams index page that performs the same action as clicking on the title of the team (a behavior that is currently too obscure as indicated in the comments on the issue). This should reduce support burden and serve as a decent temporary measure until the Add Member flow is refactored.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Oct 19, 2023
@pull-request-size pull-request-size bot added the size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. label Oct 19, 2023
@github-actions github-actions bot added the modifies/templates This PR modifies the template files label Oct 19, 2023
@tomholford tomholford marked this pull request as ready for review October 19, 2023 05:12
@Sonic853
Copy link
Contributor

I think the button here would be best changed to "View". Because this team information page can be viewed by all members of the organization, but only some members with permissions can modify team information and add members on this team information page.

@pull-request-size pull-request-size bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. labels Oct 19, 2023
@github-actions github-actions bot added modifies/translation and removed modifies/templates This PR modifies the template files labels Oct 19, 2023
@tomholford
Copy link
Contributor Author

tomholford commented Oct 19, 2023

I think the button here would be best changed to "View". Because this team information page can be viewed by all members of the organization, but only some members with permissions can modify team information and add members on this team information page.

Thanks @Sonic853 for the feedback. I have pushed a commit that changes the copy by adding new translation keys for each language. I am a native English speaker, and I am also familiar with Spanish, Portuguese, and Japanese, so I am comfortable with those translations. For the other languages, I copied the translation from view_home and removed the interpolation character (%s).

edit: per feedback from @lunny, I have removed the translations for other languages and only included English

@pull-request-size pull-request-size bot added size/XS Denotes a PR that changes 0-9 lines, ignoring generated files. and removed size/M Denotes a PR that changes 30-99 lines, ignoring generated files. labels Oct 19, 2023
@@ -16,6 +16,7 @@
<div class="ui top attached header">
<a class="text black" href="{{$.OrgLink}}/teams/{{.LowerName | PathEscape}}"><strong>{{.Name}}</strong></a>
<div class="ui right">
<a class="btn ui primary small button" href="{{$.OrgLink}}/teams/{{.LowerName | PathEscape}}">{{ctx.Locale.Tr "org.teams.view"}}</a>
Copy link
Contributor

@wxiaoguang wxiaoguang Oct 19, 2023

Choose a reason for hiding this comment

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

There is an exactly same link above: <a class="text black" href="{{$.OrgLink}}/teams/{{.LowerName | PathEscape}}">

Why duplicate?

Ideally, either make the existing link more obvious/intuitive (eg: add an icon, or a symbol), or remove the old link, only use the new link.

(just my opinion, not a blocker)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@wxiaoguang In the linked issue (#22054), multiple users (including myself) didn't realize that the title of the group is also a link. It's only visible on hover. I am not opposed to removing the link. The purpose of this PR is to make it more obvious. Will defer to you, @lunny and @Sonic853 on whether or not we should keep the link in the title.

@tomholford tomholford changed the title teams: new Add Team Member button teams: new View button Oct 20, 2023
Copy link
Contributor

@wxiaoguang wxiaoguang left a comment

Choose a reason for hiding this comment

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

Overall LGTM. Although I prefer to only keep one link on the UI, I can also accept the duplicate links. So vote my approval ahead.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Oct 20, 2023
Copy link
Member

@silverwind silverwind left a comment

Choose a reason for hiding this comment

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

Translation needs to change

@GiteaBot GiteaBot added lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Oct 20, 2023
Per the discussion on go-gitea#22054, the flow for adding a new team mamber is not intuitive for new Gitea users.

The ideal solution would be to add a new button on the Org > Members index view (see the screenshot mockup in the issue description). However, this would require a refactor of the UX for the flow. The current flow has an implicit context of which team within the org the new member is being added to. From the Members index, there is no implicit context; the flow would have to add a picker for which team the new member should be added to.

So, as a stopgap, this change simply adds a button to the Teams index page that performs the same action as clicking on the title of the team (a behavior that is currently too obscure as indicated in the comments on the issue). This should reduce support burden and serve as a decent temporary measure until the Add Member flow is refactored.
@tomholford
Copy link
Contributor Author

@silverwind @wxiaoguang Thanks for the feedback. I have addressed both requested changes in the latest commit.

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/blocked A maintainer has reservations with the PR and thus it cannot be merged labels Oct 21, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 22, 2023
@lunny lunny enabled auto-merge (squash) October 22, 2023 12:04
@lunny lunny merged commit e3afe4a into go-gitea:main Oct 22, 2023
25 checks passed
@GiteaBot GiteaBot added this to the 1.22.0 milestone Oct 22, 2023
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Oct 22, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Oct 23, 2023
* upstream/main:
  Fix incorrect ctx usage in defer function (go-gitea#27740)
  Enable followCursor for language stats bar (go-gitea#27713)
  teams: new View button (go-gitea#27685)
  fix issues in translation file (go-gitea#27699)
  Fix an indentation in the Chinese documentation of Act Runner (go-gitea#27730)
  [skip ci] Updated translations via Crowdin
  Fix org team endpoint (go-gitea#27721)
  Improve diff tree spacing (go-gitea#27714)
  refactor: make db iterate context aware (go-gitea#27710)
  [FIX] resolve confusing colors in languages stats by insert a gap (go-gitea#27704)
  Fix sticky diff header background (go-gitea#27697)
  Replace -1 with GhostUserID (go-gitea#27703)
  Clean some functions about project issue (go-gitea#27705)
@tomholford tomholford deleted the th/add-team-member-button branch October 23, 2023 02:46
fuxiaohei pushed a commit to fuxiaohei/gitea that referenced this pull request Jan 17, 2024
Per the discussion on go-gitea#22054, the flow for adding a new team member to
an org is not intuitive for new Gitea users.

The ideal solution would be to add a new button on the Org > Members
index view (see the screenshot mockup in the issue description).
However, this would require a refactor of the UX for the flow. The
current flow has an implicit context of which team within the org the
new member is being added to ('Owners' by default). From the Members
index, there is no implicit context; the flow would have to add a picker
for which team the new member should be added to.

So, as a stopgap, this change simply adds a button to the Teams index
page that performs the same action as clicking on the title of the team
(a behavior that is currently too obscure as indicated in the comments
on the issue). This should reduce support burden and serve as a decent
temporary measure until the Add Member flow is refactored.

---------

Co-authored-by: tomholford <[email protected]>
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Jan 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. modifies/translation size/XS Denotes a PR that changes 0-9 lines, ignoring generated files.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants