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.

Sign up for GitHub

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

feat: add button to quickly clear merge message #21548

Merged
merged 27 commits into from
Nov 23, 2022
Merged

feat: add button to quickly clear merge message #21548

Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
29cbfe6
feat: add button to quickly clear merge message
kolaente Oct 22, 2022
49aaa81
fix: lint
kolaente Oct 22, 2022
e88bf88
feat: add hint about what exactly will be cleared
kolaente Oct 31, 2022
f13ecdc
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 1, 2022
84a5110
Merge branch 'main' into feature/clear-merge-commit-content-button
wxiaoguang Nov 3, 2022
1f947a8
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 4, 2022
35228b5
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 8, 2022
326bf4d
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 8, 2022
1019112
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 9, 2022
8fae2fc
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 9, 2022
b1f07a7
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 9, 2022
2e43ec0
Update web_src/js/components/PullRequestMergeForm.vue
kolaente Nov 10, 2022
f44960e
Update options/locale/locale_en-US.ini
kolaente Nov 10, 2022
b8c77e7
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 11, 2022
11605de
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 11, 2022
754c4b5
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 11, 2022
b6a89b4
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 15, 2022
4e46907
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 15, 2022
f6b7142
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 19, 2022
c96e5bd
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 19, 2022
a9d0d90
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 19, 2022
33b5656
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 21, 2022
2598ba9
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 21, 2022
65322f1
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 22, 2022
ddc470e
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 22, 2022
5801938
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 23, 2022
6da4174
Merge branch 'main' into feature/clear-merge-commit-content-button
lunny Nov 23, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions options/locale/locale_en-US.ini
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1625,6 +1625,8 @@ pulls.reopened_at = `reopened this pull request <a id="%[1]s" href="#%[1]s">%[2]
pulls.merge_instruction_hint = `You can also view <a class="show-instruction">command line instructions</a>.`
pulls.merge_instruction_step1_desc = From your project repository, check out a new branch and test the changes.
pulls.merge_instruction_step2_desc = Merge the changes and update on Gitea.
pulls.clear_merge_message = Clear merge message
wxiaoguang marked this conversation as resolved.
Show resolved Hide resolved
pulls.clear_merge_message_hint = Clearing the merge message will only remove the commit message content and keep generated git trailers such as "Co-Authored-By …".

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could do with a tooltip to explain that it removes the message but not the trailers.

Copy link
Member Author

Choose a reason for hiding this comment

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

I tried adding a tooltip but it looks like this does not work within vue components. I assume because tooltips are handled by jquery/foomantic js which does not talk to vue js. Looks like the file diff list uses a workaround to make this load automatically in the vue component. This is probably something that should be cleaned up at some point so that it uses tippy's own vue plugin with proper support for directives.

I see two options on how to solve this in this PR:

  1. Copy the same workaround from the diff file list component
  2. Don't add a tooltip - maybe we could just show the explainer text below the button in a light gray?

I'd like to go with the second option.

Copy link
Contributor

Choose a reason for hiding this comment

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

wouldn't a plain old title= attribute work?

Copy link
Member Author

Choose a reason for hiding this comment

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

I've now implemented this:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

no probs

Copy link
Member Author

Choose a reason for hiding this comment

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

wouldn't a plain old title= attribute work?

Probably, yes. I didn't think about that, was focused to much on the tooltip that's used everywhere else :)

pulls.auto_merge_button_when_succeed = (When checks succeed)
pulls.auto_merge_when_succeed = Auto merge when all checks succeed
Expand Down
3 changes: 3 additions & 0 deletions templates/repo/issue/view_content/pull.tmpl
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,8 @@
'textAutoMergeButtonWhenSucceed': {{$.locale.Tr "repo.pulls.auto_merge_button_when_succeed"}},
'textAutoMergeWhenSucceed': {{$.locale.Tr "repo.pulls.auto_merge_when_succeed"}},
'textAutoMergeCancelSchedule': {{$.locale.Tr "repo.pulls.auto_merge_cancel_schedule"}},
'textClearMergeMessage': {{$.locale.Tr "repo.pulls.clear_merge_message"}},
'textClearMergeMessageHint': {{$.locale.Tr "repo.pulls.clear_merge_message_hint"}},

'canMergeNow': {{$canMergeNow}},
'allOverridableChecksOk': {{not $notAllOverridableChecksOk}},
Expand All @@ -360,6 +362,7 @@
'defaultMergeStyle': {{.MergeStyle}},
'defaultDeleteBranchAfterMerge': {{$prUnit.PullRequestsConfig.DefaultDeleteBranchAfterMerge}},
'mergeMessageFieldPlaceHolder': {{$.locale.Tr "repo.editor.commit_message_desc"}},
'defaultMergeMessage': defaultMergeMessage,

'hasPendingPullRequestMerge': {{.HasPendingPullRequestMerge}},
'hasPendingPullRequestMergeTip': {{$hasPendingPullRequestMergeTip}},
Expand Down
11 changes: 11 additions & 0 deletions web_src/js/components/PullRequestMergeForm.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,14 @@
</div>
<div class="field">
<textarea name="merge_message_field" rows="5" :placeholder="mergeForm.mergeMessageFieldPlaceHolder" v-model="mergeMessageFieldValue"/>
<template v-if="mergeMessageFieldValue !== mergeForm.defaultMergeMessage">
<button @click.prevent="clearMergeMessage" class="ui tertiary button">
{{ mergeForm.textClearMergeMessage }}
</button>
<div class="ui label"><!-- TODO: Convert to tooltip once we can use tooltips in Vue templates -->
{{ mergeForm.textClearMergeMessageHint }}
</div>
</template>
</div>
</template>

Expand Down Expand Up @@ -174,6 +182,9 @@ export default {
this.mergeStyle = name;
this.autoMergeWhenSucceed = autoMerge;
},
clearMergeMessage() {
this.mergeMessageFieldValue = this.mergeForm.defaultMergeMessage;
},
},
};
</script>
Expand Down