Refactor pull request view (6) (#37522)

Clean up legacy logic.

* Use backend logic to choose PR timeline icon color
* Always use the Vue form to merge, remove the "StillCanManualMerge" logic

web_src/css/modules/segment.css

@@ -179,13 +179,8 @@
   margin-bottom: 1rem;
 }
 
-.ui.fitted.segment:not(.horizontally) {
-  padding-top: 0;
-  padding-bottom: 0;
-}
-.ui.fitted.segment:not(.vertically) {
-  padding-left: 0;
-  padding-right: 0;
+.ui.fitted.segment {
+  padding: 0;
 }
 
 .ui.segments .segment,

web_src/css/repo.css

@@ -1929,6 +1929,7 @@ tbody.commit-list {
   max-height: 240px; /* fit exactly 6 items, commit-status-item.height * 6 */
   overflow-x: hidden;
   transition: max-height .2s;
+  width: 100%;
 }
 
 .commit-status-item {

web_src/css/shared/flex-list.css

@@ -9,7 +9,7 @@
   margin: 0;
 }
 
-/* items have dividers between them, the dividers align with items (use parent padding) */
+/* items have dividers between them, the dividers align with items */
 .flex-divided-list,
 .flex-divided-list > .item.flex-divided-list {
   display: flex;
@@ -22,6 +22,10 @@
   padding: 10px 0;
 }
 
+.flex-divided-list > .divider {
+  margin: 0;
+}
+
 .flex-divided-list > .item + .item {
   border-top: 1px solid var(--color-secondary);
 }
@@ -101,21 +105,16 @@
 }
 
 /* special rules to make the list work with existing UI elements */
-.container-divided > .flex-divided-list > .item {
-  padding-left: 1em;
+.flex-divided-list.items-px-default > .item {
+  padding-left: 1em; /* matches ".ui.segment" padding */
   padding-right: 1em;
 }
-.container-divided > .flex-divided-list > .item.flex-divided-list {
-  padding: 0;
-}
 
-.container-padded > .flex-divided-list > .item:first-child,
-.ui.segment:not(.container-divided) > .flex-divided-list > .item:first-child {
+.ui.segment:not(.fitted) > .flex-divided-list > .item:first-child {
   padding-top: 0;
 }
 
-.container-padded > .flex-divided-list > .item:last-child,
-.ui.segment:not(.container-divided) > .flex-divided-list > .item:last-child {
+.ui.segment:not(.fitted) > .flex-divided-list > .item:last-child {
   padding-bottom: 0;
 }
 
@@ -127,7 +126,3 @@
 .flex-divided-list + .divider {
   margin-top: 0;
 }
-
-.container-padded > .flex-divided-list + .divider {
-  margin-top: 10px;
-}

web_src/js/components/PullRequestMergeForm.vue

@@ -7,6 +7,8 @@ const props = defineProps<{
   mergeFormProps: any, // TODO: this is a huge object, need to be refactored in the future
 }>();
 
+const mergeStyleManuallyMerged = 'manually-merged';
+
 const mergeForm = props.mergeFormProps;
 
 const mergeTitleFieldValue = shallowRef('');
@@ -29,10 +31,17 @@ const showMergeStyleMenu = shallowRef(false);
 const showActionForm = shallowRef(false);
 
 const mergeButtonStyleClass = computed(() => {
+  if (mergeStyle.value === mergeStyleManuallyMerged) return 'red';
   if (mergeForm.allOverridableChecksOk) return 'primary';
   return autoMergeWhenSucceed.value ? 'primary' : 'red';
 });
 
+const mergeSelectStyleClass = computed(() => {
+  if (mergeForm.emptyCommit) return '';
+  if (mergeStyle.value === mergeStyleManuallyMerged) return 'red';
+  return 'primary';
+});
+
 const forceMerge = computed(() => {
   return mergeForm.canMergeNow && !mergeForm.allOverridableChecksOk;
 });
@@ -115,30 +124,32 @@ function clearMergeMessage() {
         </div>
       </template>
 
-      <div class="field" v-if="mergeStyle === 'manually-merged'">
+      <div class="field" v-if="mergeStyle === mergeStyleManuallyMerged">
         <input type="text" name="merge_commit_id" :placeholder="mergeForm.textMergeCommitId">
       </div>
 
-      <button class="ui button" :class="mergeButtonStyleClass" type="submit" name="do" :value="mergeStyle">
-        {{ mergeStyleDetail.textDoMerge }}
-        <template v-if="autoMergeWhenSucceed">
-          {{ mergeForm.textAutoMergeButtonWhenSucceed }}
-        </template>
-      </button>
+      <div class="flex-text-block tw-gap-3">
+        <button class="ui button" :class="mergeButtonStyleClass" type="submit" name="do" :value="mergeStyle">
+          {{ mergeStyleDetail.textDoMerge }}
+          <template v-if="autoMergeWhenSucceed">
+            {{ mergeForm.textAutoMergeButtonWhenSucceed }}
+          </template>
+        </button>
 
-      <button class="ui button merge-cancel" @click="toggleActionForm(false)">
-        {{ mergeForm.textCancel }}
-      </button>
+        <button class="ui button merge-cancel" type="button" @click="toggleActionForm(false)">
+          {{ mergeForm.textCancel }}
+        </button>
 
-      <div class="ui checkbox tw-ml-1" v-if="mergeForm.isPullBranchDeletable">
-        <input name="delete_branch_after_merge" type="checkbox" v-model="deleteBranchAfterMerge" id="delete-branch-after-merge">
-        <label for="delete-branch-after-merge">{{ mergeForm.textDeleteBranch }}</label>
+        <div class="ui checkbox" v-if="mergeForm.isPullBranchDeletable">
+          <input name="delete_branch_after_merge" type="checkbox" v-model="deleteBranchAfterMerge" id="delete-branch-after-merge">
+          <label for="delete-branch-after-merge">{{ mergeForm.textDeleteBranch }}</label>
+        </div>
       </div>
     </form>
 
     <div v-if="!showActionForm" class="tw-flex">
       <!-- the merge button -->
-      <div class="ui buttons merge-button" :class="[mergeForm.emptyCommit ? '' : mergeForm.allOverridableChecksOk ? 'primary' : 'red']" @click="toggleActionForm(true)">
+      <div class="ui buttons merge-button" :class="mergeSelectStyleClass" @click="toggleActionForm(true)">
         <button class="ui button">
           <svg-icon name="octicon-git-merge"/>
           <span class="button-text">

web_src/js/features/repo-issue-pull.ts

@@ -23,6 +23,7 @@ function initRepoPullRequestUpdate(el: HTMLElement) {
     }
     let data: Record<string, any> | undefined;
     try {
+      // TODO: the response is indeed not JSON, need to fix (see backend UpdatePullRequest)
       data = await response?.json(); // the response is probably not a JSON
     } catch (error) {
       console.error(error);