From 34957c5a18151164839b6ddbba8bc724f59d7484 Mon Sep 17 00:00:00 2001
From: Chocobozzz <me@florianbigard.com>
Date: Thu, 24 Oct 2024 10:07:02 +0200
Subject: [PATCH] Simplify and improve subscribe button

---
 .../shared-forms/input-text.component.scss    |  5 ++
 .../subscribe-button.component.html           | 80 +++++++++----------
 .../subscribe-button.component.scss           | 61 ++++----------
 .../subscribe-button.component.ts             | 63 ++++++++++-----
 client/src/sass/bootstrap.scss                |  7 +-
 client/src/sass/include/_mixins.scss          | 31 ++++++-
 6 files changed, 138 insertions(+), 109 deletions(-)

diff --git a/client/src/app/shared/shared-forms/input-text.component.scss b/client/src/app/shared/shared-forms/input-text.component.scss
index ae8bf5fec..8f1cadea4 100644
--- a/client/src/app/shared/shared-forms/input-text.component.scss
+++ b/client/src/app/shared/shared-forms/input-text.component.scss
@@ -9,6 +9,11 @@ input {
 
 .btn {
   @include button-with-icon(18px);
+
+  & {
+    line-height: 18px;
+    top: 0;
+  }
 }
 
 .copy-text {
diff --git a/client/src/app/shared/shared-user-subscription/subscribe-button.component.html b/client/src/app/shared/shared-user-subscription/subscribe-button.component.html
index f4abb3ff1..fb2a7011b 100644
--- a/client/src/app/shared/shared-user-subscription/subscribe-button.component.html
+++ b/client/src/app/shared/shared-user-subscription/subscribe-button.component.html
@@ -1,71 +1,63 @@
 <div
-  class="btn-group-subscribe btn-group" role="group"
-  [ngClass]="{'subscribe-button': !isAllChannelsSubscribed, 'unsubscribe-button': isAllChannelsSubscribed, 'big': isBigButton }"
+  class="btn-group" role="group"
+  [ngClass]="{ 'big': isBigButton }"
+  ngbDropdown="dropdown" autoClose="outside" placement="bottom-right bottom-left bottom auto"
+  role="group" aria-label="Multiple ways to subscribe to the current channel" i18n-aria-label
 >
 
-  <ng-template #userLoggedOut>
-    <div [ngClass]="{ 'extra-text': isAtLeastOneChannelSubscribed }">
+  <ng-template #subscribeContent>
+    <div class="d-inline-block" [ngClass]="{ 'extra-text': isAtLeastOneChannelSubscribed }">
       @if (isSingleSubscribe) {
         <ng-container i18n>Subscribe</ng-container>
       } @else {
         <div i18n>Subscribe to all channels</div>
 
-        <div class="mt-1 fs-8" *ngIf="isAtLeastOneChannelSubscribed" i18n>
+        <div class="fs-8 fw-normal lh-1" *ngIf="isAtLeastOneChannelSubscribed" i18n>
           {{ subscribeStatus(true).length }}/{{ subscribed.size }} channels subscribed
         </div>
       }
     </div>
-
-    <span *ngIf="!isBigButton && displayFollowers && videoChannels.length > 1 && videoChannel.followersCount !== 0" class="followers-count">
-      {{ videoChannels[0].followersCount | myNumberFormatter }}
-    </span>
   </ng-template>
 
-  <ng-template #userLoggedIn>
+  @if (isUserLoggedIn()) {
     @if (isAllChannelsSubscribed) {
-      <button type="button" class="btn" role="button" (click)="unsubscribe()">
+      <button type="button" [ngClass]="buttonClasses" class="btn" (click)="unsubscribe()">
         <ng-container i18n>{account + "", select, undefined {Unsubscribe} other {Unsubscribe from all channels}}</ng-container>
       </button>
     } @else {
-      <button type="button" class="btn" (click)="subscribe()">
-        <ng-template [ngTemplateOutlet]="userLoggedOut"></ng-template>
+      <button type="button" [ngClass]="buttonClasses" class="btn" (click)="subscribe()">
+        <ng-template [ngTemplateOutlet]="subscribeContent"></ng-template>
       </button>
+
+      <button type="button" [ngClass]="buttonClasses" class="btn dropdown-toggle-split" ngbDropdownToggle aria-label="Open subscription dropdown" i18n-ariaLabel></button>
     }
-  </ng-template>
+  } @else {
+    <button ngbDropdownToggle [ngClass]="buttonClasses" type="button" class="btn">
+      <ng-template [ngTemplateOutlet]="subscribeContent"></ng-template>
+    </button>
+  }
 
-  <ng-container
-    *ngIf="isUserLoggedIn(); then userLoggedIn">
-  </ng-container>
+  <div class="dropdown-menu" ngbDropdownMenu>
 
-  <div
-    class="btn-group" ngbDropdown autoClose="outside" placement="bottom-right bottom-left bottom auto"
-    role="group" aria-label="Multiple ways to subscribe to the current channel" i18n-aria-label
-  >
-    <button class="btn dropdown-toggle-split last-in-group" ngbDropdownToggle aria-label="Open subscription dropdown" i18n-aria-label>
-      <ng-container
-        *ngIf="!isUserLoggedIn(); then userLoggedOut">
-      </ng-container>
+    <h6 class="dropdown-header" i18n>Using an ActivityPub account</h6>
+
+    <button type="button" class="dropdown-item" (click)="subscribe()">
+      @if (isUserLoggedIn()) {
+        <span i18n>Subscribe with your local account</span>
+      } @else {
+        <span i18n>Subscribe with an account on this instance</span>
+      }
     </button>
 
-    <div class="dropdown-menu" ngbDropdownMenu>
-
-      <h6 class="dropdown-header" i18n>Using an ActivityPub account</h6>
-
-      <button class="dropdown-item" (click)="subscribe()">
-        <span *ngIf="!isUserLoggedIn()" i18n>Subscribe with an account on this instance</span>
-        <span *ngIf="isUserLoggedIn()" i18n>Subscribe with your local account</span>
-      </button>
-
-      <button *ngIf="isRemoteSubscribeAvailable()" class="dropdown-item dropdown-item-neutral">
-        <div class="mb-1" i18n>Subscribe with a remote account:</div>
-        <my-remote-subscribe [showHelp]="true" [uri]="uri"></my-remote-subscribe>
-      </button>
-
-      <div class="dropdown-divider"></div>
-
-      <h6 class="dropdown-header" i18n>Using a syndication feed</h6>
-      <a [href]="rssUri" target="_blank" class="dropdown-item" i18n>Subscribe via RSS</a>
-
+    <div type="button" *ngIf="isRemoteSubscribeAvailable()" class="dropdown-item dropdown-item-neutral">
+      <div class="mb-1" i18n>Subscribe with a remote account:</div>
+      <my-remote-subscribe [showHelp]="true" [uri]="uri"></my-remote-subscribe>
     </div>
+
+    <div class="dropdown-divider"></div>
+
+    <h6 class="dropdown-header" i18n>Using a syndication feed</h6>
+    <a [href]="rssUri" target="_blank" class="dropdown-item" i18n>Subscribe via RSS</a>
+
   </div>
 </div>
diff --git a/client/src/app/shared/shared-user-subscription/subscribe-button.component.scss b/client/src/app/shared/shared-user-subscription/subscribe-button.component.scss
index 55a9b2f7b..f1d0fd0bd 100644
--- a/client/src/app/shared/shared-user-subscription/subscribe-button.component.scss
+++ b/client/src/app/shared/shared-user-subscription/subscribe-button.component.scss
@@ -1,14 +1,26 @@
 @use '_variables' as *;
 @use '_mixins' as *;
 
-.btn-group-subscribe {
+.btn-group {
   padding: 0;
 
-  @include peertube-button;
+  .peertube-button {
+    // Prevent weird border radius blur on chrome
+    z-index: unset !important;
+  }
 
-  button.dropdown-toggle {
-    font-size: $button-font-size;
-    line-height: 1.2;
+  .dropdown-toggle::after {
+    position: relative;
+    top: 1px;
+  }
+
+  .dropdown-toggle:not(.dropdown-toggle-split)::after {
+    margin-left: 0.5rem;
+  }
+
+  .dropdown-toggle-split {
+    padding-left: 8px;
+    padding-right: 8px;
   }
 
   &:not(.big) {
@@ -20,45 +32,6 @@
     min-width: 175px;
   }
 
-  // Unlogged
-  > .dropdown > .dropdown-toggle span {
-    @include padding-right(5px);
-  }
-
-  // Logged
-  > .btn {
-    @include padding-right(4px);
-
-    + .dropdown > button {
-      @include padding-left(2px);
-
-      &::after {
-        position: relative;
-        top: 1px;
-      }
-    }
-  }
-
-  &.subscribe-button {
-    .btn {
-      font-weight: 600;
-
-      @include orange-button;
-    }
-
-    span.followers-count {
-      @include padding-left(5px);
-    }
-  }
-
-  &.unsubscribe-button {
-    .btn {
-      font-weight: 600;
-
-      @include grey-button;
-    }
-  }
-
   .dropdown-menu {
     cursor: default;
 
diff --git a/client/src/app/shared/shared-user-subscription/subscribe-button.component.ts b/client/src/app/shared/shared-user-subscription/subscribe-button.component.ts
index 4c66483ed..325e77f88 100644
--- a/client/src/app/shared/shared-user-subscription/subscribe-button.component.ts
+++ b/client/src/app/shared/shared-user-subscription/subscribe-button.component.ts
@@ -1,15 +1,15 @@
-import { concat, forkJoin, merge } from 'rxjs'
-import { Component, Input, OnChanges, OnInit } from '@angular/core'
-import { AuthService, Notifier, RedirectService } from '@app/core'
-import { FeedFormat } from '@peertube/peertube-models'
-import { UserSubscriptionService } from './user-subscription.service'
-import { NumberFormatterPipe } from '../shared-main/common/number-formatter.pipe'
-import { RemoteSubscribeComponent } from './remote-subscribe.component'
-import { NgbDropdown, NgbDropdownToggle, NgbDropdownMenu } from '@ng-bootstrap/ng-bootstrap'
 import { NgClass, NgIf, NgTemplateOutlet } from '@angular/common'
-import { VideoChannel } from '../shared-main/channel/video-channel.model'
+import { Component, Input, OnChanges, ViewChild } from '@angular/core'
+import { AuthService, Notifier, RedirectService } from '@app/core'
+import { NgbDropdown, NgbDropdownItem, NgbDropdownMenu, NgbDropdownToggle } from '@ng-bootstrap/ng-bootstrap'
+import { FeedFormat } from '@peertube/peertube-models'
+import { concat, forkJoin, merge } from 'rxjs'
 import { Account } from '../shared-main/account/account.model'
+import { VideoChannel } from '../shared-main/channel/video-channel.model'
+import { NumberFormatterPipe } from '../shared-main/common/number-formatter.pipe'
 import { VideoService } from '../shared-main/video/video.service'
+import { RemoteSubscribeComponent } from './remote-subscribe.component'
+import { UserSubscriptionService } from './user-subscription.service'
 
 @Component({
   selector: 'my-subscribe-button',
@@ -23,11 +23,12 @@ import { VideoService } from '../shared-main/video/video.service'
     NgbDropdown,
     NgbDropdownToggle,
     NgbDropdownMenu,
+    NgbDropdownItem,
     RemoteSubscribeComponent,
     NumberFormatterPipe
   ]
 })
-export class SubscribeButtonComponent implements OnInit, OnChanges {
+export class SubscribeButtonComponent implements OnChanges {
   /**
    * SubscribeButtonComponent can be used with a single VideoChannel passed as [VideoChannel],
    * or with an account and a full list of that account's videoChannels. The latter is intended
@@ -36,11 +37,14 @@ export class SubscribeButtonComponent implements OnInit, OnChanges {
    */
   @Input() account: Account
   @Input() videoChannels: VideoChannel[]
-  @Input() displayFollowers = false
   @Input() size: 'small' | 'normal' = 'normal'
 
+  @ViewChild('dropdown') dropdown: NgbDropdown
+
   subscribed = new Map<string, boolean>()
 
+  buttonClasses: Record<string, boolean> = {}
+
   constructor (
     private authService: AuthService,
     private redirectService: RedirectService,
@@ -97,15 +101,16 @@ export class SubscribeButtonComponent implements OnInit, OnChanges {
     return !this.account
   }
 
-  ngOnInit () {
+  ngOnChanges () {
     this.loadSubscribedStatus()
+    this.buildClasses()
   }
 
-  ngOnChanges () {
-    this.ngOnInit()
-  }
+  // ---------------------------------------------------------------------------
 
   subscribe () {
+    if (this.dropdown) this.dropdown.close()
+
     if (this.isUserLoggedIn()) {
       return this.localSubscribe()
     }
@@ -113,7 +118,7 @@ export class SubscribeButtonComponent implements OnInit, OnChanges {
     return this.gotoLogin()
   }
 
-  localSubscribe () {
+  private localSubscribe () {
     const subscribedStatus = this.subscribeStatus(false)
 
     const observableBatch = this.videoChannels
@@ -124,6 +129,8 @@ export class SubscribeButtonComponent implements OnInit, OnChanges {
     forkJoin(observableBatch)
       .subscribe({
         next: () => {
+          this.buildClasses()
+
           this.notifier.success(
             this.account
               ? $localize`Subscribed to all current channels of ${this.account.displayName}. You will be notified of all their new videos.`
@@ -137,13 +144,17 @@ export class SubscribeButtonComponent implements OnInit, OnChanges {
       })
   }
 
+  // ---------------------------------------------------------------------------
+
   unsubscribe () {
+    if (this.dropdown) this.dropdown.close()
+
     if (this.isUserLoggedIn()) {
       this.localUnsubscribe()
     }
   }
 
-  localUnsubscribe () {
+  private localUnsubscribe () {
     const subscribeStatus = this.subscribeStatus(true)
 
     const observableBatch = this.videoChannels
@@ -154,6 +165,8 @@ export class SubscribeButtonComponent implements OnInit, OnChanges {
     concat(...observableBatch)
       .subscribe({
         complete: () => {
+          this.buildClasses()
+
           this.notifier.success(
             this.account
               ? $localize`Unsubscribed from all channels of ${this.account.nameWithHost}`
@@ -167,6 +180,8 @@ export class SubscribeButtonComponent implements OnInit, OnChanges {
       })
   }
 
+  // ---------------------------------------------------------------------------
+
   isUserLoggedIn () {
     return this.authService.isLoggedIn()
   }
@@ -207,10 +222,22 @@ export class SubscribeButtonComponent implements OnInit, OnChanges {
         this.userSubscriptionService.listenToSubscriptionCacheChange(handle),
         this.userSubscriptionService.doesSubscriptionExist(handle)
       ).subscribe({
-        next: res => this.subscribed.set(handle, res),
+        next: res => {
+          this.subscribed.set(handle, res)
+
+          this.buildClasses()
+        },
 
         error: err => this.notifier.error(err.message)
       })
     }
   }
+
+  private buildClasses () {
+    this.buttonClasses = {
+      'peertube-button': true,
+      'orange-button': !this.isAllChannelsSubscribed,
+      'grey-button': this.isAllChannelsSubscribed
+    }
+  }
 }
diff --git a/client/src/sass/bootstrap.scss b/client/src/sass/bootstrap.scss
index b8f47d2be..f38e3c981 100644
--- a/client/src/sass/bootstrap.scss
+++ b/client/src/sass/bootstrap.scss
@@ -56,6 +56,12 @@ body {
   --bs-btn-color: #{pvar(--greyForegroundColor)};
 }
 
+.btn {
+  --bs-btn-active-color: inherit;
+  --bs-btn-active-bg: inherit;
+  --bs-btn-active-border-color: inherit;
+}
+
 .flex-auto {
   flex: auto;
 }
@@ -291,7 +297,6 @@ body {
 
 .btn:not(.btn-sm) {
   font-size: $button-font-size;
-  line-height: 1.2;
 }
 
 .btn-outline-secondary {
diff --git a/client/src/sass/include/_mixins.scss b/client/src/sass/include/_mixins.scss
index 1420b8a9d..87fb476db 100644
--- a/client/src/sass/include/_mixins.scss
+++ b/client/src/sass/include/_mixins.scss
@@ -127,6 +127,16 @@
   &:focus {
     color: #fff;
     background-color: pvar(--mainColor);
+    border-color: inherit;
+  }
+
+  // Override bootstrap
+  &.btn:active,
+  &.btn:focus-visible,
+  &.btn.show {
+    color: #fff !important;
+    background-color: pvar(--mainColor) !important;
+    border-color: inherit !important;
   }
 
   &:hover {
@@ -159,6 +169,15 @@
     background-color: pvar(--mainBackgroundColor);
   }
 
+  // Override bootstrap
+  &.btn:active,
+  &.btn:focus-visible,
+  &.btn.show {
+    color: pvar(--mainColor);
+    background-color: pvar(--mainBackgroundColor);
+    border-color: inherit !important;
+  }
+
   &:hover {
     color: pvar(--mainColor);
     background-color: pvar(--mainColorLightest);
@@ -204,6 +223,15 @@
     background-color: pvar(--greySecondaryBackgroundColor);
   }
 
+  // Override bootstrap
+  &.btn:active,
+  &.btn:focus-visible,
+  &.btn.show {
+    color: pvar(--greyForegroundColor);
+    background-color: pvar(--greySecondaryBackgroundColor);
+    border-color: inherit !important;
+  }
+
   &[disabled] {
     cursor: default;
   }
@@ -241,8 +269,7 @@
   border: 0;
   font-weight: $font-semibold;
 
-  // Because of primeng that redefines border-radius of all input[type="..."]
-  border-radius: 3px !important;
+  border-radius: 3px;
 
   text-align: center;
   cursor: pointer;