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

add disable workflow feature #26413

Merged
merged 23 commits into from
Aug 14, 2023

Conversation

a1012112796
Copy link
Member

As title, that's simmilar with github.

image

image

Signed-off-by: a1012112796 <1012112796@qq.com>
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 9, 2023
@pull-request-size pull-request-size bot added the size/L Denotes a PR that changes 100-499 lines, ignoring generated files. label Aug 9, 2023
@a1012112796 a1012112796 added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Aug 9, 2023
@silverwind
Copy link
Member

silverwind commented Aug 9, 2023

Button looks misaligned, but I guess I can help with styling later. I would also put it into a "..." menu because it should probably not be such a prominent button.

models/repo/repo_unit.go Outdated Show resolved Hide resolved
routers/web/repo/actions/view.go Outdated Show resolved Hide resolved
@wolfogre wolfogre added the topic/gitea-actions related to the actions of Gitea label Aug 10, 2023
@wolfogre wolfogre added this to the 1.21.0 milestone Aug 10, 2023
@yp05327
Copy link
Contributor

yp05327 commented Aug 10, 2023

How about only add a toggle switch here:
image
Instead of Enable/Disable button here, it seems not clear which workflow to Enable/Disable and you need to select this workflow in the list first.
image

Of cause, putting it into a "..." menu here also looks good.

@silverwind
Copy link
Member

UI might look a bit cramped with switches in the workflow list, and I do see a certain benefit of having the button in the same place as GH.

Copy link
Member

@wolfogre wolfogre left a comment

Choose a reason for hiding this comment

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

LGTM on the backend part.

@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 Aug 11, 2023
@silverwind
Copy link
Member

silverwind commented Aug 11, 2023

Did some UI enhancements. Menu button that submits a hidden form, red label (imho warranted to have this call to attention via color):

Screenshot 2023-08-11 at 08 07 08 Screenshot 2023-08-11 at 08 07 19

The flashes sent by the backend do not work for me, even before my changes, so those need to be fixed or removed before merge.

@silverwind
Copy link
Member

silverwind commented Aug 12, 2023

Regarding flash, I'm pretty sure we are missing the flash subtemplate somewhere. We should refactor the templates to ensure this subtemplate is included in every page.

@silverwind
Copy link
Member

Mobile view fixed:

Screenshot 2023-08-12 at 12 21 24

There was a small issue that the margin was too little caused by Fomantic's negative margins on .ui.secondary.menu. I have set these to zero now and checked various pages and it seems fine. On issue list, removing the negative margin also fixes this misalignment:

Before:
Screenshot 2023-08-12 at 12 17 39

After:
Screenshot 2023-08-12 at 12 17 32

web_src/css/base.css Outdated Show resolved Hide resolved
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.

Looks good, waiting on @wxiaoguang for #26413 (comment).

@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Aug 14, 2023
@wxiaoguang
Copy link
Contributor

Looks good, waiting on @wxiaoguang for #26413 (comment).

Although I haven't tested nor fully understood, I think it might be not bad, so no objection.

@silverwind
Copy link
Member

I have tested it across many pages, no issues with the menu changes, so I'm pretty sure they are safe.

@silverwind silverwind added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 14, 2023
@silverwind silverwind enabled auto-merge (squash) August 14, 2023 14:25
@silverwind silverwind merged commit 1987206 into go-gitea:main Aug 14, 2023
23 checks passed
@GiteaBot GiteaBot removed the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Aug 14, 2023
zjjhot added a commit to zjjhot/gitea that referenced this pull request Aug 15, 2023
* upstream/main:
  Use `object-fit: contain` for oauth2 custom icons (go-gitea#26493)
  add disable workflow feature (go-gitea#26413)
  Move dropzone progress bar to bottom to show filename when uploading (go-gitea#26492)
  Handle base64 decoding correctly to avoid panic (go-gitea#26483)
  Allow to archive labels (go-gitea#26478)
@a1012112796 a1012112796 deleted the zzc/dev/disable_workflow branch August 16, 2023 04:26
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Nov 13, 2023
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. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. topic/gitea-actions related to the actions of Gitea type/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants