From ce28993c78ae9a964b84f21d6000978f15237560 Mon Sep 17 00:00:00 2001 From: Jamie Kyle <113370520+jamiebuilds-signal@users.noreply.github.com> Date: Thu, 10 Aug 2023 13:40:05 -0700 Subject: [PATCH] Fix sticker pack preview modal scrollbars and modal scroll dividers --- stylesheets/components/Modal.scss | 21 +++++++++++++----- stylesheets/components/StickerManager.scss | 4 +--- ts/components/AddCaptionModal.tsx | 25 +++++++++------------- ts/components/Modal.tsx | 4 ++++ ts/hooks/useSizeObserver.tsx | 9 ++++---- 5 files changed, 35 insertions(+), 28 deletions(-) diff --git a/stylesheets/components/Modal.scss b/stylesheets/components/Modal.scss index 30581d077..567f4fe16 100644 --- a/stylesheets/components/Modal.scss +++ b/stylesheets/components/Modal.scss @@ -125,10 +125,10 @@ &--has-header#{&}--header-divider { .module-Modal__body { @include light-theme() { - border-top-color: $color-gray-15; + border-top-color: $color-gray-05; } @include dark-theme() { - border-top-color: $color-gray-60; + border-top-color: $color-gray-90; } } } @@ -137,6 +137,7 @@ .module-Modal__body { padding-top: 0; border-top: 1px solid transparent; + border-bottom: 1px solid transparent; &--scrolled { @include light-theme { @@ -144,7 +145,7 @@ } @include dark-theme { - border-top-color: $color-gray-80; + border-top-color: $color-gray-90; } } } @@ -153,10 +154,10 @@ &--has-footer#{&}--footer-divider { .module-Modal__body { @include light-theme() { - border-bottom-color: $color-gray-15; + border-bottom-color: $color-gray-05; } @include dark-theme() { - border-bottom-color: $color-gray-60; + border-bottom-color: $color-gray-90; } } } @@ -164,6 +165,16 @@ &--has-footer { .module-Modal__body { border-bottom: 1px solid transparent; + + &--overflow:not(&--scrolledToBottom) { + @include light-theme { + border-bottom-color: $color-gray-05; + } + + @include dark-theme { + border-bottom-color: $color-gray-90; + } + } } } diff --git a/stylesheets/components/StickerManager.scss b/stylesheets/components/StickerManager.scss index 38b409466..053e7e04f 100644 --- a/stylesheets/components/StickerManager.scss +++ b/stylesheets/components/StickerManager.scss @@ -217,8 +217,7 @@ .module-sticker-manager__preview-modal { &__modal.module-Modal { - max-width: 440px; - width: 440px; + width: fit-content; } &__error { color: $color-accent-red; @@ -239,7 +238,6 @@ display: grid; grid-gap: 8px; grid-template-columns: repeat(4, 1fr); - overflow-y: auto; &__cell { width: 96px; diff --git a/ts/components/AddCaptionModal.tsx b/ts/components/AddCaptionModal.tsx index 24ea2ba73..8eed5bc4b 100644 --- a/ts/components/AddCaptionModal.tsx +++ b/ts/components/AddCaptionModal.tsx @@ -1,13 +1,14 @@ // Copyright 2022 Signal Messenger, LLC // SPDX-License-Identifier: AGPL-3.0-only -import React, { useEffect } from 'react'; +import React from 'react'; import { noop } from 'lodash'; import { Button } from './Button'; import { Modal } from './Modal'; import type { LocalizerType, ThemeType } from '../types/Util'; import type { SmartCompositionTextAreaProps } from '../state/smart/CompositionTextArea'; import type { HydratedBodyRangesType } from '../types/BodyRange'; +import { isScrolled, isScrolledToBottom } from '../hooks/useSizeObserver'; export type Props = { i18n: LocalizerType; @@ -38,8 +39,9 @@ export function AddCaptionModal({ HydratedBodyRangesType | undefined >(); - const [isScrolledTop, setIsScrolledTop] = React.useState(true); - const [isScrolledBottom, setIsScrolledBottom] = React.useState(true); + const [scrolled, setScrolled] = React.useState(false); + // We don't know that this is true, but it most likely is + const [scrolledToBottom, setScrolledToBottom] = React.useState(true); const scrollerRef = React.useRef(null); @@ -47,17 +49,10 @@ export function AddCaptionModal({ const updateScrollState = React.useCallback(() => { const scrollerEl = scrollerRef.current; if (scrollerEl) { - setIsScrolledTop(scrollerEl.scrollTop === 0); - setIsScrolledBottom( - scrollerEl.scrollHeight - scrollerEl.scrollTop === - scrollerEl.clientHeight - ); + setScrolled(isScrolled(scrollerEl)); + setScrolledToBottom(isScrolledToBottom(scrollerEl)); } - }, [scrollerRef]); - - useEffect(() => { - updateScrollState(); - }, [updateScrollState]); + }, []); const handleSubmit = React.useCallback(() => { onSubmit(messageText, bodyRanges); @@ -68,8 +63,8 @@ export function AddCaptionModal({ i18n={i18n} modalName="AddCaptionModal" hasXButton - hasHeaderDivider={!isScrolledTop} - hasFooterDivider={!isScrolledBottom} + hasHeaderDivider={scrolled} + hasFooterDivider={!scrolledToBottom} moduleClassName="AddCaptionModal" padded={false} title={i18n('icu:AddCaptionModal__title')} diff --git a/ts/components/Modal.tsx b/ts/components/Modal.tsx index 5bba0990a..f8c379418 100644 --- a/ts/components/Modal.tsx +++ b/ts/components/Modal.tsx @@ -18,6 +18,7 @@ import * as log from '../logging/log'; import { isOverflowing, isScrolled, + isScrolledToBottom, useScrollObserver, } from '../hooks/useSizeObserver'; @@ -175,6 +176,7 @@ export function ModalPage({ const bodyInnerRef = useRef(null); const [scrolled, setScrolled] = useState(false); + const [scrolledToBottom, setScrolledToBottom] = useState(false); const [hasOverflow, setHasOverflow] = useState(false); const hasHeader = Boolean(hasXButton || title || onBackButtonClick); @@ -182,6 +184,7 @@ export function ModalPage({ useScrollObserver(bodyRef, bodyInnerRef, scroll => { setScrolled(isScrolled(scroll)); + setScrolledToBottom(isScrolledToBottom(scroll)); setHasOverflow(isOverflowing(scroll)); }); @@ -250,6 +253,7 @@ export function ModalPage({ className={classNames( getClassName('__body'), scrolled ? getClassName('__body--scrolled') : null, + scrolledToBottom ? getClassName('__body--scrolledToBottom') : null, hasOverflow || scrolled ? getClassName('__body--overflow') : null )} ref={bodyRef} diff --git a/ts/hooks/useSizeObserver.tsx b/ts/hooks/useSizeObserver.tsx index 024730d93..ccee503ff 100644 --- a/ts/hooks/useSizeObserver.tsx +++ b/ts/hooks/useSizeObserver.tsx @@ -93,11 +93,10 @@ export function SizeObserver({ return children(ref, size); } -export type Scroll = Readonly<{ - scrollTop: number; - scrollHeight: number; - clientHeight: number; -}>; +// Note: You should just be able to pass an element into utils if you want. +export type Scroll = Readonly< + Pick +>; export type ScrollChangeHandler = (scroll: Scroll) => void;