change update visiblity ux #25856
change update visiblity ux #25856
Conversation
GiteaBot
added
the
lgtm/need 2
pull-request-size
bot
added
the
size/L
|
Can you add also screenshot how it looks to make it public |
|
make repository private modal |
|
when the repository is already private you can make it public like before. without any warning
there is no section in danger zone |
|
Imho, we should completely move it to danger zone under label "Change repository visibility" there, e.g. remove the old checkbox and this tooltip that sounds like broken english and make change in both directions only via danger zone. |
|
at first, I want to do so. but is it really dangerous to make a repository public?? I mean warnings are just reasonable when you want to make it private. |
lunny
added
the
topic/ui-interaction
|
It is potentially dangerous to make a repo public, as if you have private code (or perhaps infrastructure as code templates) that contain sensitive information then you wouldn't want that out in the world. |
|
There could be an easier approach: When toggling the "checkbox", show a modal dialog "Do you want to make the repo public/priate? It will cause (1) (2) (3) ... [Cancel] [Confirm]", if "cancel", then do not toggle the "checked" state. Then:
|
|
I would like this PR to split the private/public switch operations with other settings changes. The private/public switching operations should be a heavy operation which need to change many databases records. |
|
I still prefer to have it in the danger zone only because both direction can be considered dangerous under certain conditions and it's consistent with GitHub UI. Public -> Private may break dependents (for example golang), Private -> Public may expose unintended information to the public. |
|
totally move it to danger zone also looks good to me |
GiteaBot
added
lgtm/blocked
|
based on feedback changing visibility moved to the danger zone
when repo is public when repo is private |
# Conflicts: # templates/repo/settings/options.tmpl
|
@mohammad258852 Could you update your branch and resolve merge conflicts? |
|
Also, since your PR, the layout changed a bit. You'd need to update your PR to the new layout. |
Move red text to paragraph above and remove parens. 
Uncolor text and add a "." at the end. Imho, there is no reason to style this entry differently then the rest. Just make it consistent with existing entries. Danger zone implies danger. |
|
branch is now up-to-date. |
denyskon
added
the
giteabot/update-branch
GiteaBot
removed
the
giteabot/update-branch
| </div> | ||
| <div class="required field"> | ||
| <label for="repo_name">{{ctx.Locale.Tr "repo.repo_name"}}</label> | ||
| <input id="repo_name" name="repo_name" required> |
There was a problem hiding this comment.
There are many id="repo_name" elements in this page. The "id" should be unique.
| @@ -763,6 +756,44 @@ | |||
| </div> | |||
| </div> | |||
| {{end}} | |||
|
|
|||
| {{if not .Repository.IsFork}} | |||
| <div class="divider"></div> | |||
There was a problem hiding this comment.
Why this divider (and below) is needed? IIRC the flex-item has proper border-top: 1px solid var(--color-secondary);.
| } | ||
|
|
||
| visibilityChanged := repo.IsPrivate != private | ||
| if visibilityChanged && setting.Repository.ForcePrivate && !private && !ctx.Doer.IsAdmin { |
There was a problem hiding this comment.
A useful comment is lost
// when ForcePrivate enabled, you could change public repo to private, but only admin users can change private to public
| if !ctx.Repo.IsOwner() { | ||
| ctx.Error(http.StatusNotFound) | ||
| return | ||
| } |
There was a problem hiding this comment.
The old code doesn't have such check.
IIRC the IsOwner should have been correctly checked by a middleware for this handler.
|
replaced by #31126 |
Fixes: #25085
Fixes: #23826
before
after
change UX for update visibility. when the Repository is visible, in the basic settings section, the user can just see the visibility status. For changing visibility, he/she should go to the danger zone section. if the Repository is not visible, use can use basic settings section.