-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
ui: add more message on sidebar menus #10872
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
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.
Apart from comment re: nothing to show looks good
routers/repo/compare.go
Outdated
@@ -434,6 +434,8 @@ func CompareDiff(ctx *context.Context) { | |||
setTemplateIfExists(ctx, pullRequestTemplateKey, pullRequestTemplateCandidates) | |||
renderAttachmentSettings(ctx) | |||
|
|||
ctx.Data["IsIssueWriter"] = ctx.Repo.CanWrite(models.UnitTypePullRequests) |
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.
We should rename it as IsPullRequestWriter
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.
Hello, your suggestion is reasonable, but maybe not necessary, the main problem is that the new issue and new Pull Request use same tmpl new_form.tmpl
, If change it , the tmpl will become complex. then we have think of Pull Request as issue in many place , so it's not a big problem. Thanks.
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.
This could also be ctx.Data["HasWritePermissions"] too which I think is more clear
IsIssueWriter or IsPullRequesttWriter makes it sound like that user wrote/authored the issue or pull request when really we are just checking if they have write permissions)
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.
This could also be ctx.Data["HasWritePermissions"] too which I think is more clear
IsIssueWriter or IsPullRequesttWriter makes it sound like that user wrote/authored the issue or pull request when really we are just checking if they have write permissions)
that's great but maybe not accurate enough, how about HasIssuesOrPullsWritePermission
?
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.
Sure, that would work fine too I think
6f9947e
to
525b123
Compare
* add title on the menus * show some message instead of hide choose bar when have nothing to choose * add simply filter for each menus * do same changes in mew_form.tmpl * remove some unusefull comments in mew_form.tmpl Signed-off-by: a1012112796 <[email protected]>
Hello, How about this change, looking forward for response ... |
templates/repo/issue/new_form.tmpl
Outdated
{{if or .Labels .OrgLabels}} | ||
<div class="ui icon search input"> | ||
<i class="search icon"></i> | ||
<input type="text" placeholder="..."> |
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.
These should all be translatable strings that say "Filter Label", "Filter Milestone" etc..
Other than that looks great to me ty <3 |
Hello, I'm sorry, but I can't understand your meaning, Could you please explain it in detail? Thanks. |
I mean aside from my request to add translated strings to the place holder values the PR looks good and I would approve once those changes are made. |
* add filter message on sidebar filter * change IsIssueWriter to HasIssuesOrPullsWritePermission
Codecov Report
@@ Coverage Diff @@
## master #10872 +/- ##
==========================================
+ Coverage 43.61% 43.62% +0.01%
==========================================
Files 597 597
Lines 83919 83923 +4
==========================================
+ Hits 36600 36615 +15
+ Misses 42808 42798 -10
+ Partials 4511 4510 -1
Continue to review full report at Codecov.
|
screenshots:
