Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new CanvasProvider state to manage the opening of the ThumbsPagesPod accordion #622

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

sergioelmoreno
Copy link
Collaborator

When the user load a saved file, the ThumbsPagesPod accordion is open by default to improve the User Experience

There is no issue related

@sergioelmoreno sergioelmoreno self-assigned this Dec 17, 2024
@sergioelmoreno sergioelmoreno changed the title Add new CanvasProvider state to manage the openinig of the ThumbsPagesPod accordion Add new CanvasProvider state to manage the opening of the ThumbsPagesPod accordion Dec 17, 2024
@@ -25,6 +25,7 @@ export const CanvasProvider: React.FC<Props> = props => {
const [fileName, setFileName] = React.useState<string>('');
const [isThumbnailContextMenuVisible, setIsThumbnailContextMenuVisible] =
React.useState(false);
const [isFileLoaded, setIsFileLoaded] = React.useState(false);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A flag is a good idea, but true/false won't work the second time you open a document, one trick we can do is to count the number of times we have opened documents, something like:

const [howManyLoadedDocuments, setHowManyLoadedDocuments] = React.useState(0);

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The setter we can call it directly here in the provider (no need to call it from outside, something like:

  const loadDocument = (document: DocumentModel) => {
    setDocument(document);
+    setHowManyLoadedDocuments(numberOfDocuments => numberOfDocuments + 1);
  };

@@ -72,6 +72,7 @@ export const useLocalDisk = () => {
}
};
reader.readAsText(file);
setIsFileLoaded(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather in the provider (see comment)


export const MainScene = () => {
const { isThumbPagesPodOpen, thumbPagesPodRef } =
useAccordionSectionVisibility();

const { isFileLoaded } = useCanvasContext();
const forceOpenThumbsPages = isFileLoaded && { open: true };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

mmm.. we could create a custom hook to encapsulate the open doc funciontallity, the use it directly in the scene, something like:

import { useCanvasContext } from '@/core/providers';
import { useEffect, useRef, useState } from 'react';

export const useAccordionSectionVisibility = () => {
  const [isThumbPagesPodOpen, setIsThumbPagesPodOpen] = useState(false);
  const thumbPagesPodRef = useRef<HTMLDetailsElement>(null);
  const { fullDocument, howManyLoadedDocuments } = useCanvasContext();

  useEffect(() => {
    if (
      howManyLoadedDocuments > 0 &&
      thumbPagesPodRef.current &&
      fullDocument.pages.length > 1
    ) {
      setIsThumbPagesPodOpen(true);
      thumbPagesPodRef.current.open = true;
    }
  }, [howManyLoadedDocuments]);

  useEffect(() => {
    const handleToggle = () => {
      setIsThumbPagesPodOpen(thumbPagesPodRef.current?.open ?? false);
    };

    const detailsElement = thumbPagesPodRef.current;
    if (detailsElement) {
      detailsElement.addEventListener('toggle', handleToggle);
    }

    // Cleanup event listener on component unmount
    return () => {
      if (detailsElement) {
        detailsElement.removeEventListener('toggle', handleToggle);
      }
    };
  }, []);

  return {
    thumbPagesPodRef,
    isThumbPagesPodOpen,
  };
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request needs discussion
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants