Merge pull request 'Improve git notes UI' (#6025) from 0ko/forgejo:ui-notes-followup into forgejo

Reviewed-on: https://codeberg.org/forgejo/forgejo/pulls/6025
Reviewed-by: Michael Kriese <michael.kriese@gmx.de>
Reviewed-by: Earl Warren <earl-warren@noreply.codeberg.org>

options/locale/locale_en-US.ini

@@ -2622,9 +2622,9 @@ diff.browse_source = Browse source
 diff.parent = parent
 diff.commit = commit
 diff.git-notes = Notes
-diff.git-notes.add = Add Note
-diff.git-notes.remove-header = Remove Note
-diff.git-notes.remove-body = This will remove this Note
+diff.git-notes.add = Add note
+diff.git-notes.remove-header = Remove note
+diff.git-notes.remove-body = This note will be removed.
 diff.data_not_available = Diff content is not available
 diff.options_button = Diff options
 diff.show_diff_stats = Show stats

package-lock.json

@@ -18,7 +18,7 @@
         "asciinema-player": "3.8.0",
         "chart.js": "4.4.5",
         "chartjs-adapter-dayjs-4": "1.0.4",
-        "chartjs-plugin-zoom": "2.0.1",
+        "chartjs-plugin-zoom": "2.1.0",
         "clippie": "4.1.1",
         "css-loader": "7.0.0",
         "dayjs": "1.11.12",
@@ -4807,6 +4807,12 @@
       "integrity": "sha512-WCfD5Ht3ZesJUsONdhvm84dmzWOiOzOAqOncN0++w0lBw1o8OuDNJF2McvvCef/yBqb/HYRahp1BYtODFQ8bRg==",
       "license": "MIT"
     },
+    "node_modules/@types/hammerjs": {
+      "version": "2.0.46",
+      "resolved": "https://registry.npmjs.org/@types/hammerjs/-/hammerjs-2.0.46.tgz",
+      "integrity": "sha512-ynRvcq6wvqexJ9brDMS4BnBLzmr0e14d6ZJTEShTBWKymQiHwlAyGu0ZPEFI2Fh1U53F7tN9ufClWM5KvqkKOw==",
+      "license": "MIT"
+    },
     "node_modules/@types/json-schema": {
       "version": "7.0.15",
       "resolved": "https://registry.npmjs.org/@types/json-schema/-/json-schema-7.0.15.tgz",
@@ -6479,11 +6485,12 @@
       }
     },
     "node_modules/chartjs-plugin-zoom": {
-      "version": "2.0.1",
-      "resolved": "https://registry.npmjs.org/chartjs-plugin-zoom/-/chartjs-plugin-zoom-2.0.1.tgz",
-      "integrity": "sha512-ogOmLu6e+Q7E1XWOCOz9YwybMslz9qNfGV2a+qjfmqJYpsw5ZMoRHZBUyW+NGhkpQ5PwwPA/+rikHpBZb7PZuA==",
+      "version": "2.1.0",
+      "resolved": "https://registry.npmjs.org/chartjs-plugin-zoom/-/chartjs-plugin-zoom-2.1.0.tgz",
+      "integrity": "sha512-7lMimfQCUaIJLhPJaWSAA4gw+1m8lyR3Wn+M3MxjHbM/XxRUnOxN7cM5RR9jUmxmyW0h7L2hZ8KhvUsqrFxy/Q==",
       "license": "MIT",
       "dependencies": {
+        "@types/hammerjs": "^2.0.45",
         "hammerjs": "^2.0.8"
       },
       "peerDependencies": {

package.json

@@ -17,7 +17,7 @@
     "asciinema-player": "3.8.0",
     "chart.js": "4.4.5",
     "chartjs-adapter-dayjs-4": "1.0.4",
-    "chartjs-plugin-zoom": "2.0.1",
+    "chartjs-plugin-zoom": "2.1.0",
     "clippie": "4.1.1",
     "css-loader": "7.0.0",
     "dayjs": "1.11.12",

release-notes-assistant.sh

@@ -7,6 +7,7 @@ label_bug=bug
 label_feature=feature
 label_ui=forgejo/ui
 label_breaking=breaking
+label_security=security
 label_localization=forgejo/i18n
 
 payload=$(mktemp)
@@ -17,50 +18,71 @@ function test_main() {
   set -ex
   PS4='${BASH_SOURCE[0]}:$LINENO: ${FUNCNAME[0]}:  '
 
+  test_payload_labels $label_worth $label_breaking $label_security $label_bug
+  test "$(categorize)" = 'AA Breaking security bug fixes'
+
+  test_payload_labels $label_worth $label_security $label_bug
+  test "$(categorize)" = 'AB Security bug fixes'
+
+  test_payload_labels $label_worth $label_breaking $label_security $label_feature
+  test "$(categorize)" = 'AC Breaking security features'
+
+  test_payload_labels $label_worth $label_security $label_feature
+  test "$(categorize)" = 'AD Security features'
+
+  test_payload_labels $label_worth $label_security
+  test "$(categorize)" = 'ZA Security changes without a feature or bug label'
+
   test_payload_labels $label_worth $label_breaking $label_feature
-  test "$(categorize)" = 'AA Breaking features'
+  test "$(categorize)" = 'BA Breaking features'
 
   test_payload_labels $label_worth $label_breaking $label_bug
-  test "$(categorize)" = 'AB Breaking bug fixes'
+  test "$(categorize)" = 'BB Breaking bug fixes'
 
   test_payload_labels $label_worth $label_breaking
-  test "$(categorize)" = 'ZC Breaking changes without a feature or bug label'
+  test "$(categorize)" = 'ZB Breaking changes without a feature or bug label'
 
   test_payload_labels $label_worth $label_ui $label_feature
-  test "$(categorize)" = 'BA User Interface features'
+  test "$(categorize)" = 'CA User Interface features'
 
   test_payload_labels $label_worth $label_ui $label_bug
-  test "$(categorize)" = 'BB User Interface bug fixes'
+  test "$(categorize)" = 'CB User Interface bug fixes'
 
   test_payload_labels $label_worth $label_ui
-  test "$(categorize)" = 'ZD User Interface changes without a feature or bug label'
-
-  test_payload_labels $label_worth $label_feature
-  test "$(categorize)" = 'CA Features'
-
-  test_payload_labels $label_worth $label_bug
-  test "$(categorize)" = 'CB Bug fixes'
+  test "$(categorize)" = 'ZC User Interface changes without a feature or bug label'
 
   test_payload_labels $label_worth $label_localization
   test "$(categorize)" = 'DA Localization'
 
+  test_payload_labels $label_worth $label_feature
+  test "$(categorize)" = 'EA Features'
+
+  test_payload_labels $label_worth $label_bug
+  test "$(categorize)" = 'EB Bug fixes'
+
   test_payload_labels $label_worth
   test "$(categorize)" = 'ZE Other changes without a feature or bug label'
 
   test_payload_labels
   test "$(categorize)" = 'ZF Included for completeness but not worth a release note'
 
+  test_payload_draft "fix(security)!: breaking security bug fix"
+  test "$(categorize)" = 'AA Breaking security bug fixes'
+
+  test_payload_draft "fix(security): security bug fix"
+  test "$(categorize)" = 'AB Security bug fixes'
+
   test_payload_draft "feat!: breaking feature"
-  test "$(categorize)" = 'AA Breaking features'
+  test "$(categorize)" = 'BA Breaking features'
 
   test_payload_draft "fix!: breaking bug fix"
-  test "$(categorize)" = 'AB Breaking bug fixes'
+  test "$(categorize)" = 'BB Breaking bug fixes'
 
   test_payload_draft "feat: feature"
-  test "$(categorize)" = 'CA Features'
+  test "$(categorize)" = 'EA Features'
 
   test_payload_draft "fix: bug fix"
-  test "$(categorize)" = 'CB Bug fixes'
+  test "$(categorize)" = 'EB Bug fixes'
 
   test_payload_draft "something with no prefix"
   test "$(categorize)" = 'ZE Other changes without a feature or bug label'
@@ -109,6 +131,7 @@ function categorize() {
   is_feature=false
   is_localization=false
   is_breaking=false
+  is_security=false
 
   #
   # first try to figure out the category from the labels
@@ -125,6 +148,12 @@ function categorize() {
     ;;
   esac
 
+  case "$labels" in
+  *$label_security*)
+    is_security=true
+    ;;
+  esac
+
   case "$labels" in
   *$label_breaking*)
     is_breaking=true
@@ -143,6 +172,15 @@ function categorize() {
   if ! $is_bug && ! $is_feature; then
     draft="$(jq --raw-output .Draft <$payload)"
     case "$draft" in
+    fix\(security\)!:*)
+      is_bug=true
+      is_breaking=true
+      is_security=true
+      ;;
+    fix\(security\):*)
+      is_bug=true
+      is_security=true
+      ;;
     fix!:*)
       is_bug=true
       is_breaking=true
@@ -171,29 +209,45 @@ function categorize() {
     fi
   fi
 
-  if $is_breaking; then
-    if $is_feature; then
-      echo -n AA Breaking features
-    elif $is_bug; then
-      echo -n AB Breaking bug fixes
+  if $is_security; then
+    if $is_bug; then
+      if $is_breaking; then
+        echo -n AA Breaking security bug fixes
+      else
+        echo -n AB Security bug fixes
+      fi
+    elif $is_feature; then
+      if $is_breaking; then
+        echo -n AC Breaking security features
+      else
+        echo -n AD Security features
+      fi
     else
-      echo -n ZC Breaking changes without a feature or bug label
+      echo -n ZA Security changes without a feature or bug label
+    fi
+  elif $is_breaking; then
+    if $is_feature; then
+      echo -n BA Breaking features
+    elif $is_bug; then
+      echo -n BB Breaking bug fixes
+    else
+      echo -n ZB Breaking changes without a feature or bug label
     fi
   elif $is_ui; then
     if $is_feature; then
-      echo -n BA User Interface features
+      echo -n CA User Interface features
     elif $is_bug; then
-      echo -n BB User Interface bug fixes
+      echo -n CB User Interface bug fixes
     else
-      echo -n ZD User Interface changes without a feature or bug label
+      echo -n ZC User Interface changes without a feature or bug label
     fi
   elif $is_localization; then
     echo -n DA Localization
   else
     if $is_feature; then
-      echo -n CA Features
+      echo -n EA Features
     elif $is_bug; then
-      echo -n CB Bug fixes
+      echo -n EB Bug fixes
     else
       echo -n ZE Other changes without a feature or bug label
     fi

templates/repo/commit_page.tmpl

@@ -128,6 +128,9 @@
 											</form>
 										</div>
 									</div>
+									<div id="commit-notes-add-button" class="item">
+										{{ctx.Locale.Tr "repo.diff.git-notes.add"}}
+									</div>
 								</div>
 							</div>
 						{{end}}
@@ -277,20 +280,20 @@
 				<span class="text grey" id="note-authored-time">{{DateUtils.TimeSince .NoteCommit.Author.When}}</span>
 				{{if or ($.Permission.CanWrite $.UnitTypeCode) (not $.Repository.IsArchived) (not .IsDeleted)}}
 					<div class="ui right">
-						<button id="commit-notes-edit-button" class="ui tiny button yellow" data-modal="#delete-note-modal">{{ctx.Locale.Tr "edit"}}</button>
+						<button id="commit-notes-edit-button" class="ui tiny primary button" data-modal="#delete-note-modal">{{ctx.Locale.Tr "edit"}}</button>
 						<button class="ui tiny button red show-modal" data-modal="#delete-note-modal">{{ctx.Locale.Tr "remove"}}</button>
 					</div>
 					<div class="ui small modal" id="delete-note-modal">
 						<div class="header">
 							{{ctx.Locale.Tr "repo.diff.git-notes.remove-header"}}
 						</div>
-						<p>{{ctx.Locale.Tr "repo.diff.git-notes.remove-body"}}</p>
 						<div class="content">
+							<p>{{ctx.Locale.Tr "repo.diff.git-notes.remove-body"}}</p>
 							<div class="text right actions">
 								<form action="{{.Link}}/notes/remove" method="post">
 									{{.CsrfTokenHtml}}
 									<button type="button" class="ui cancel button">{{ctx.Locale.Tr "settings.cancel"}}</button>
-									<button type="submit" class="ui primary button red" href="{{.Link}}/notes/remove">{{ctx.Locale.Tr "remove"}}</button>
+									<button type="submit" class="ui red button" href="{{.Link}}/notes/remove">{{ctx.Locale.Tr "remove"}}</button>
 								</form>
 							</div>
 						</div>
@@ -316,7 +319,6 @@
 				</div>
 			{{end}}
 		{{else if and ($.Permission.CanWrite $.UnitTypeCode) (not $.Repository.IsArchived) (not .IsDeleted)}}
-			<button id="commit-notes-add-button" class="ui primary button green tw-mt-3">{{ctx.Locale.Tr "repo.diff.git-notes.add"}}</button>
 			<div id="commit-notes-add-area" class="ui tw-mt-3 segment tw-hidden">
 				<form class="ui form" action="{{.Link}}/notes" method="post">
 					{{.CsrfTokenHtml}}

web_src/js/features/repo-commit.js

@@ -38,7 +38,6 @@ export function initCommitNotes() {
   const notesAddButton = document.getElementById('commit-notes-add-button');
   if (notesAddButton !== null) {
     notesAddButton.addEventListener('click', () => {
-      notesAddButton.classList.add('tw-hidden');
       document.getElementById('commit-notes-add-area').classList.remove('tw-hidden');
     });
   }