Pinned messages UI fixes

stylesheets/_modules.scss

@@ -1874,83 +1874,19 @@ $message-padding-horizontal: 12px;
   display: inline-block;
   margin-inline-start: 6px;
   margin-bottom: 2px;
-
-  & {
-    @include mixins.dark-theme {
-      @include mixins.color-svg(
-        '../images/icons/v3/message_timer/messagetimer-60.svg',
-        variables.$color-white-alpha-80
-      );
-    }
-  }
-  & {
-    @include mixins.light-theme {
-      @include mixins.color-svg(
-        '../images/icons/v3/message_timer/messagetimer-60.svg',
-        variables.$color-white-alpha-80
-      );
-    }
-  }
 }
 
 $timer-icons:
-  '55', '50', '45', '40', '35', '30', '25', '20', '15', '10', '05', '00';
+  '60', '55', '50', '45', '40', '35', '30', '25', '20', '15', '10', '05', '00';
 
 @each $timer-icon in $timer-icons {
   .module-expire-timer--#{$timer-icon} {
-    & {
-      @include mixins.dark-theme {
     @include mixins.color-svg(
       '../images/icons/v3/message_timer/messagetimer-#{$timer-icon}.svg',
-          variables.$color-white-alpha-80
+      currentColor
     );
   }
 }
-    & {
-      @include mixins.light-theme {
-        @include mixins.color-svg(
-          '../images/icons/v3/message_timer/messagetimer-#{$timer-icon}.svg',
-          variables.$color-white-alpha-80
-        );
-      }
-    }
-  }
-}
-
-.module-expire-timer--incoming {
-  background-color: variables.$color-white-alpha-80;
-
-  @include mixins.light-theme {
-    background-color: variables.$color-gray-60;
-  }
-  @include mixins.dark-theme {
-    background-color: variables.$color-gray-25;
-  }
-}
-.module-expire-timer--with-sticker {
-  @include mixins.light-theme {
-    background-color: variables.$color-gray-60;
-  }
-}
-
-// When status indicators are overlaid on top of an image, they use different colors
-.module-expire-timer--with-image-no-caption {
-  @include mixins.light-theme {
-    background-color: variables.$color-white;
-  }
-  @include mixins.dark-theme {
-    background-color: variables.$color-gray-02;
-  }
-}
-
-.module-expire-timer--outline-only-bubble {
-  @include mixins.light-theme {
-    background-color: variables.$color-gray-60;
-  }
-  @include mixins.dark-theme {
-    background-color: variables.$color-gray-25;
-  }
-}
 
 .module-about {
   &__container {

ts/components/conversation/ExpireTimer.dom.stories.tsx

@@ -11,12 +11,9 @@ export default {
 } satisfies Meta<Props>;
 
 const createProps = (overrideProps: Partial<Props> = {}): Props => ({
-  direction: overrideProps.direction || 'outgoing',
   expirationLength: overrideProps.expirationLength || 30 * 1000,
   expirationTimestamp:
     overrideProps.expirationTimestamp || Date.now() + 30 * 1000,
-  withImageNoCaption: overrideProps.withImageNoCaption || false,
-  withSticker: overrideProps.withSticker || false,
 });
 
 export const _30Seconds = (): React.JSX.Element => {
@@ -51,38 +48,6 @@ export function Expired(): React.JSX.Element {
   return <ExpireTimer {...props} />;
 }
 
-export function Sticker(): React.JSX.Element {
-  const props = createProps({
-    withSticker: true,
-  });
-
-  return <ExpireTimer {...props} />;
-}
-
-export function ImageNoCaption(): React.JSX.Element {
-  const props = createProps({
-    withImageNoCaption: true,
-  });
-
-  return (
-    <div style={{ backgroundColor: 'darkgreen' }}>
-      <ExpireTimer {...props} />
-    </div>
-  );
-}
-
-export function Incoming(): React.JSX.Element {
-  const props = createProps({
-    direction: 'incoming',
-  });
-
-  return (
-    <div style={{ backgroundColor: 'darkgreen' }}>
-      <ExpireTimer {...props} />
-    </div>
-  );
-}
-
 export function ExpirationTooFarOut(): React.JSX.Element {
   const props = createProps({
     expirationTimestamp: Date.now() + 150 * 1000,

ts/components/conversation/ExpireTimer.dom.tsx

@@ -6,22 +6,14 @@ import classNames from 'classnames';
 
 import { getIncrement, getTimerBucket } from '../../util/timer.std.js';
 
-export type Props = {
+export type Props = Readonly<{
-  direction?: 'incoming' | 'outgoing';
   expirationLength: number;
   expirationTimestamp?: number;
-  isOutlineOnlyBubble?: boolean;
+}>;
-  withImageNoCaption?: boolean;
-  withSticker?: boolean;
-};
 
 export function ExpireTimer({
-  direction,
   expirationLength,
   expirationTimestamp,
-  isOutlineOnlyBubble,
-  withImageNoCaption,
-  withSticker,
 }: Props): React.JSX.Element {
   const [, forceUpdate] = useReducer(() => ({}), {});
 
@@ -40,13 +32,7 @@ export function ExpireTimer({
     <div
       className={classNames(
         'module-expire-timer',
-        `module-expire-timer--${bucket}`,
+        `module-expire-timer--${bucket}`
-        direction ? `module-expire-timer--${direction}` : null,
-        isOutlineOnlyBubble ? 'module-expire-timer--outline-only-bubble' : null,
-        direction && withImageNoCaption
-          ? 'module-expire-timer--with-image-no-caption'
-          : null,
-        withSticker ? 'module-expire-timer--with-sticker' : null
       )}
     />
   );

ts/components/conversation/MessageMetadata.dom.tsx

@@ -220,12 +220,8 @@ export const MessageMetadata = forwardRef<HTMLDivElement, Readonly<PropsType>>(
         ) : null}
         {expirationLength ? (
           <ExpireTimer
-            direction={metadataDirection}
-            isOutlineOnlyBubble={isOutlineOnlyBubble}
             expirationLength={expirationLength}
             expirationTimestamp={expirationTimestamp}
-            withImageNoCaption={withImageNoCaption}
-            withSticker={isSticker}
           />
         ) : null}
         {textPending ? (

ts/components/conversation/Quote.dom.stories.tsx

@@ -77,7 +77,7 @@ const defaultMessageProps: TimelineMessagesProps = {
   canEditMessage: true,
   canEndPoll: false,
   canForward: true,
-  canPinMessages: true,
+  canPinMessage: true,
   canReact: true,
   canReply: true,
   canRetry: true,

ts/components/conversation/Timeline.dom.stories.tsx

@@ -47,7 +47,7 @@ function mockMessageTimelineItem(
       canEditMessage: true,
       canEndPoll: false,
       canForward: true,
-      canPinMessages: true,
+      canPinMessage: true,
       canReact: true,
       canReply: true,
       canRetry: true,

ts/components/conversation/TimelineMessage.dom.stories.tsx

@@ -239,7 +239,7 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
   canCopy: true,
   canEditMessage: true,
   canEndPoll: overrideProps.direction === 'outgoing',
-  canPinMessages: overrideProps.canPinMessages ?? true,
+  canPinMessage: overrideProps.canPinMessage ?? true,
   canReact: true,
   canReply: true,
   canDownload: true,

ts/components/conversation/TimelineMessage.dom.tsx

@@ -54,7 +54,7 @@ export type PropsData = {
   canRetryDeleteForEveryone: boolean;
   canReact: boolean;
   canReply: boolean;
-  canPinMessages: boolean;
+  canPinMessage: boolean;
   hasMaxPinnedMessages: boolean;
   selectedReaction?: string;
   isTargeted?: boolean;
@@ -114,7 +114,7 @@ export function TimelineMessage(props: Props): React.JSX.Element {
     canReply,
     canRetry,
     canRetryDeleteForEveryone,
-    canPinMessages,
+    canPinMessage,
     containerElementRef,
     containerWidthBreakpoint,
     conversationId,
@@ -368,11 +368,9 @@ export function TimelineMessage(props: Props): React.JSX.Element {
             });
           }}
           onPinMessage={
-            canPinMessages && !isPinned ? handleOpenPinMessageDialog : null
+            canPinMessage && !isPinned ? handleOpenPinMessageDialog : null
-          }
-          onUnpinMessage={
-            canPinMessages && isPinned ? handleUnpinMessage : null
           }
+          onUnpinMessage={canPinMessage && isPinned ? handleUnpinMessage : null}
           onMoreInfo={() =>
             pushPanelForConversation({
               type: PanelType.MessageDetails,
@@ -388,7 +386,7 @@ export function TimelineMessage(props: Props): React.JSX.Element {
       canCopy,
       canEditMessage,
       canForward,
-      canPinMessages,
+      canPinMessage,
       canRetry,
       canSelect,
       canEndPoll,

ts/components/conversation/pinned-messages/PinnedMessagesBar.dom.stories.tsx

@@ -182,10 +182,28 @@ export function Variants(): React.JSX.Element {
         title="Poll"
         message={{ poll: { question: `${SHORT_TEXT}?` } }}
       />
-      <Variant title="Sticker" message={{ sticker: true }} />
+      <Variant
-      <Variant title="Contact" message={{ contact: { name: 'Tyler' } }} />
+        title="Sticker"
+        message={{
+          sticker: true,
+          attachment: { type: 'image', url: IMAGE_URL },
+        }}
+      />
+      <Variant
+        title="Contact"
+        message={{
+          contact: { name: 'Tyler' },
+          attachment: { type: 'file', name: '' },
+        }}
+      />
       <Variant title="Payment" message={{ payment: true }} />
-      <Variant title="View-Once Media" message={{ viewOnceMedia: true }} />
+      <Variant
+        title="View-Once Media"
+        message={{
+          viewOnceMedia: true,
+          attachment: { type: 'image', url: IMAGE_URL },
+        }}
+      />
     </Stack>
   );
 }

ts/components/conversation/pinned-messages/PinnedMessagesBar.dom.tsx

@@ -191,7 +191,7 @@ function TabsList(props: {
   return (
     <AriaClickable.SubWidget>
       <Tabs.List className={tw('flex h-full flex-col')}>
-        {props.pins.toReversed().map((pin, pinIndex) => {
+        {props.pins.map((pin, pinIndex) => {
           return (
             <TabTrigger
               key={pin.id}
@@ -395,7 +395,39 @@ function getMessagePreview(i18n: LocalizerType, message: PinMessage): Preview {
     text ??= <MessageTextPreview i18n={i18n} text={message.text} />;
   }
 
-  // 3. And everything else...
+  // 3. Check specific types of messages (before checking attachments)
+  if (message.payment) {
+    text ??= i18n('icu:PinnedMessagesBar__MessagePreview__Text--Payment');
+    icon ??= {
+      symbol: 'creditcard',
+      label: i18n(
+        'icu:PinnedMessagesBar__MessagePreview__SymbolLabel--Payment'
+      ),
+    };
+  } else if (message.poll != null) {
+    text ??= <UserText text={message.poll.question} />;
+    icon ??= {
+      symbol: 'poll',
+      label: i18n('icu:PinnedMessagesBar__MessagePreview__SymbolLabel--Poll'),
+    };
+  } else if (message.sticker) {
+    text ??= i18n('icu:PinnedMessagesBar__MessagePreview__Text--Sticker');
+    icon ??= {
+      symbol: 'sticker',
+      label: i18n(
+        'icu:PinnedMessagesBar__MessagePreview__SymbolLabel--Sticker'
+      ),
+    };
+  } else if (message.contact != null) {
+    const name = message.contact.name ?? i18n('icu:unknownContact');
+    text ??= <UserText text={name} />;
+    icon ??= {
+      symbol: 'person-circle',
+      label: name,
+    };
+  }
+
+  // 4. Check attachments (make sure to check types like sticker/contact first)
   if (message.attachment != null) {
     if (message.attachment.type === 'image') {
       text ??= i18n('icu:PinnedMessagesBar__MessagePreview__Text--Photo');
@@ -426,35 +458,6 @@ function getMessagePreview(i18n: LocalizerType, message: PinMessage): Preview {
     } else {
       throw missingCaseError(message.attachment);
     }
-  } else if (message.contact != null) {
-    const name = message.contact.name ?? i18n('icu:unknownContact');
-    text ??= <UserText text={name} />;
-    icon ??= {
-      symbol: 'person-circle',
-      label: name,
-    };
-  } else if (message.payment) {
-    text ??= i18n('icu:PinnedMessagesBar__MessagePreview__Text--Payment');
-    icon ??= {
-      symbol: 'creditcard',
-      label: i18n(
-        'icu:PinnedMessagesBar__MessagePreview__SymbolLabel--Payment'
-      ),
-    };
-  } else if (message.poll != null) {
-    text ??= <UserText text={message.poll.question} />;
-    icon ??= {
-      symbol: 'poll',
-      label: i18n('icu:PinnedMessagesBar__MessagePreview__SymbolLabel--Poll'),
-    };
-  } else if (message.sticker) {
-    text ??= i18n('icu:PinnedMessagesBar__MessagePreview__Text--Sticker');
-    icon ??= {
-      symbol: 'sticker',
-      label: i18n(
-        'icu:PinnedMessagesBar__MessagePreview__SymbolLabel--Sticker'
-      ),
-    };
   }
 
   return { icon, text };

ts/messageModifiers/PinnedMessages.preload.ts

@@ -16,6 +16,7 @@ import { pinnedMessagesCleanupService } from '../services/expiring/pinnedMessage
 import { drop } from '../util/drop.std.js';
 import type { AppendPinnedMessageResult } from '../sql/server/pinnedMessages.std.js';
 import * as Errors from '../types/errors.std.js';
+import { isGiftBadge } from '../state/selectors/message.preload.js';
 
 const { AccessRequired } = Proto.AccessControl;
 const { Role } = Proto.Member;
@@ -209,6 +210,8 @@ function validatePinnedMessageTarget(
   target: MessageModifierTarget,
   sourceAci: AciString
 ): { error: string } | null {
+  const message = target.targetMessage.attributes;
+
   if (!isValidSenderAciForConversation(target.targetConversation, sourceAci)) {
     return { error: 'Sender cannot send to target conversation' };
   }
@@ -217,5 +220,9 @@ function validatePinnedMessageTarget(
     return { error: 'Sender does not have access to edit group attributes' };
   }
 
+  if (isGiftBadge(message)) {
+    return { error: 'Cannot pin gift badge messages' };
+  }
+
   return null;
 }

ts/sql/server/pinnedMessages.std.ts

@@ -64,7 +64,7 @@ export function getPinnedMessagesPreloadDataForConversation(
     const [query, params] = sql`
       SELECT * FROM pinnedMessages
       WHERE conversationId = ${conversationId}
-      ORDER BY pinnedAt DESC
+      ORDER BY pinnedAt ASC
     `;
 
     return db

ts/state/selectors/message.preload.ts

@@ -946,7 +946,7 @@ export const getPropsForMessage = (
     canDownload: canDownload(message, conversationSelector),
     canEndPoll: canEndPoll(message),
     canForward: canForward(message),
-    canPinMessages: canPinMessages(conversation),
+    canPinMessage: canPinMessage(conversation, message),
     canReact: canReact(message, ourConversationId, conversationSelector),
     canReply: canReply(message, ourConversationId, conversationSelector),
     canRetry: hasErrors(message),
@@ -2414,6 +2414,19 @@ export function canPinMessages(conversation: ConversationType): boolean {
   return conversation.type === 'direct' || canEditGroupInfo(conversation);
 }
 
+export function canPinMessage(
+  conversation: ConversationType,
+  message: ReadonlyMessageAttributesType
+): boolean {
+  if (!canPinMessages(conversation)) {
+    return false;
+  }
+  if (isGiftBadge(message)) {
+    return false;
+  }
+  return true;
+}
+
 function getHasMaxPinnedMessages(
   pinnedMessagesMessageIds: ReadonlyArray<string>
 ) {

ts/state/smart/PinnedMessagesBar.preload.tsx

@@ -132,6 +132,10 @@ function getPinSender(props: MessagePropsType): PinSender {
   };
 }
 
+function getLastPinId(pins: ReadonlyArray<Pin>): PinnedMessageId | null {
+  return pins.at(-1)?.id ?? null;
+}
+
 function getPrevPinId(
   pins: ReadonlyArray<Pin>,
   pinnedMessageId: PinnedMessageId
@@ -375,26 +379,45 @@ export const SmartPinnedMessagesBar = memo(function SmartPinnedMessagesBar() {
     useConversationsActions();
 
   const [current, setCurrent] = useState(() => {
-    return pins.at(0)?.id ?? null;
+    return getLastPinId(pins);
   });
 
-  const isCurrentOutOfDate = useMemo(() => {
+  const [prevPins, setPrevPins] = useState(pins);
+  if (pins !== prevPins) {
+    // Needed for `expectedCurrent` which might update `current` in the same render
+    setPrevPins(pins);
+  }
+
+  const expectedCurrent = useMemo(() => {
+    const latestPinId = getLastPinId(pins);
+
+    // If `current` is null, use the latest pin id if we have one.
     if (current == null) {
-      if (pins.length > 0) {
+      return latestPinId;
-        return true;
-      }
-      return false;
     }
 
-    const hasMatch = pins.some(pin => {
+    // If `current` is already the latest pin id, leave it.
-      return pin.id === current;
+    if (current === latestPinId) {
-    });
+      return current;
+    }
 
-    return !hasMatch;
+    // Update `current` if it no longer exists.
-  }, [current, pins]);
+    const hasCurrent = pins.some(pin => pin.id === current);
+    if (!hasCurrent) {
+      return latestPinId;
+    }
 
-  if (isCurrentOutOfDate) {
+    // Update `current` if it was previously the latest and there's a new latest.
-    setCurrent(pins.at(0)?.id ?? null);
+    const prevLatestPinId = getLastPinId(prevPins);
+    if (prevLatestPinId === current && latestPinId != null) {
+      return latestPinId;
+    }
+
+    return current;
+  }, [current, pins, prevPins]);
+
+  if (current !== expectedCurrent) {
+    setCurrent(expectedCurrent);
   }
 
   const handleCurrentChange = useCallback(
@@ -410,9 +433,10 @@ export const SmartPinnedMessagesBar = memo(function SmartPinnedMessagesBar() {
       if (current == null) {
         return;
       }
-      const prevPinId = getPrevPinId(pins, current);
+
-      if (prevPinId != null) {
+      const updatedCurrent = getPrevPinId(pins, current) ?? getLastPinId(pins);
-        setCurrent(prevPinId);
+      if (updatedCurrent != null) {
+        setCurrent(updatedCurrent);
       }
     },
     [scrollToMessage, conversationId, pins, current]