-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
Fix "delete" modal dialog for issue/PR #27015
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
Conversation
| .ui.modal > .actions, | ||
| .ui.modal .content + .actions { | ||
| .ui.modal .content + .actions, | ||
| .ui.modal .content + form > .actions { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are these fomantic's selectors or our own? If our own, maybe we can simplify.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see, well it could be simplified to .content .actions or even .ui.modal .actions, but leaving that up to you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I do not feel it is safe to use content or actions without a strict rule. It would cause pollution because many one-word classes (including "content") have been used everywhere.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Another (strong) reason is that the content itself has paddings, so the actions inside it shouldn't have padding again. But the actions following it should still have the padding. Line 60 comment: /* if the "actions" is in the "content", some paddings are already added by the "content" */
Declaring these style explicitly is good for maintainability.
* upstream/main: Add more package registry paths to the labeler (go-gitea#27032) Extract auth middleware from service (go-gitea#27028) S3: log human readable error on connection failure (go-gitea#26856) [skip ci] Updated translations via Crowdin Fix "delete" modal dialog for issue/PR (go-gitea#27015) Fix context cache bug & enable context cache for dashabord commits' authors (go-gitea#26991) fix: typo (go-gitea#27009) Use secure cookie for HTTPS sites (go-gitea#26999) Add fetch wrappers, ignore network errors in actions view (go-gitea#26985)

Close #27012
By the way, rename the single-word ID to a long ID.