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

[Data Explorer][Discover 2.0] Full screen view is hiding under the nav #4824

Open
ananzh opened this issue Aug 25, 2023 · 2 comments
Open

[Data Explorer][Discover 2.0] Full screen view is hiding under the nav #4824

ananzh opened this issue Aug 25, 2023 · 2 comments
Labels
data explorer Issues related to the Data Explorer project discover for discover reinvent

Comments

@ananzh
Copy link
Member

ananzh commented Aug 25, 2023

image (9)

@ananzh ananzh added discover for discover reinvent data explorer Issues related to the Data Explorer project v2.10.0 labels Aug 25, 2023
@ananzh ananzh self-assigned this Aug 25, 2023
@ananzh ananzh removed the untriaged label Aug 25, 2023
@ananzh
Copy link
Member Author

ananzh commented Aug 25, 2023

Issue

After investigation, I think this is due to no easy way to listen to or detect when the data grid goes into fullscreen mode. In OuiDataGrid, it sets this isFullScreen property internally

const [isFullScreen, setIsFullScreen] = useState(false);

Then the Full screen button/link is using this fullScreenSelector where onClick can catch the click event to update isFullScreen.

const fullScreenSelector = (
    <OuiI18n
      tokens={[
        'ouiDataGrid.fullScreenButton',
        'ouiDataGrid.fullScreenButtonActive',
      ]}
      defaults={['Full screen', 'Exit full screen']}>
      {([fullScreenButton, fullScreenButtonActive]: ReactChild[]) => (
        <OuiButtonEmpty
          size="xs"
          iconType="fullScreen"
          color="text"
          className={controlBtnClasses}
          data-test-subj="dataGridFullScrenButton"
          onClick={() => setIsFullScreen(!isFullScreen)}>
          {isFullScreen ? fullScreenButtonActive : fullScreenButton}
        </OuiButtonEmpty>
      )}
    </OuiI18n>
  );

Then isFullScreen property will be passed to OuiDataGridBody where window height is defined as

if (isFullScreen) {
    finalHeight =
      window.innerHeight - toolbarHeight - headerRowHeight - footerRowHeight;
    finalWidth = window.innerWidth;
  }

andtoolbarHeight and headerRowHeight are not controlled by us.

Therefore, when user clicks Full screen, only OuiDataGrid catch it. We won’t be able to catch the click in our application in OpenSearch Dashboards. I was hoping to find some way to detect whether there is a full screen. Then by detecting full screen mode and have some property like showTopNavMenu to update TopNavMenu config, like

   <TopNavMenu
      .. other property
      config={showTopNavMenu ? topNavMenu : undefined}
    />`

Options

Option 1 use DOM Mutation Observers

Use a MutationObserver to detect when the OuiDataGrid or a specific child element of it receives a class indicating it's in fullscreen mode. I do see the wrapper class changed from <div class=" euiDataGrid euiDataGrid--bordersAll euiDataGrid--headerShade euiDataGrid--footerOverline euiDataGrid--rowHoverHighlight euiDataGrid--stickyFooter" data-test-subj="docTable"> to <div class=" euiDataGrid euiDataGrid--bordersAll euiDataGrid--headerShade euiDataGrid--footerOverline euiDataGrid--rowHoverHighlight euiDataGrid--stickyFooter **euiDataGrid--fullScreen**" data-test-subj="docTable">, and I hope we could capture the change. I have updated our table comp to

// add prop to detect comp updates
const [gridNode, setGridNode] = useState(null);
const dataGridRef = useRef(null);
// detect comp changes
useEffect(() => {
    setGridNode(dataGridRef.current);
   }, [dataGridRef]);

  useEffect(() => {
    // Check if the browser supports MutationObserver
    if ('MutationObserver' in window && dataGridRef.current) {
        // Initialize observer
        const observer = new MutationObserver(mutations => {
            mutations.forEach(mutation => {
                if (mutation.type === 'attributes' && mutation.attributeName === 'class') {
                    const target = mutation.target as HTMLElement;
                    if (target.classList.contains('euiDataGrid--fullScreen')) {
                        console.log('The table has entered full-screen mode');
                    } else {
                        console.log('The table has not entered full-screen mode');
                    }
                }
            });
        });

        // Start the observation
        observer.observe(dataGridRef.current, {
            attributes: true, // Observe changes in attributes
            attributeFilter: ['class'], // Specifically, the class attribute
        });

        // Clean-up function to stop observing
        return () => {
            observer.disconnect();
        };
    }
}, [gridNode]);

// 
const table = useMemo(
    () => (
     <div ref={dataGridRef}>
       <EuiDataGrid
        aria-labelledby="aria-labelledby"
        columns={dataGridTableColumns}
        columnVisibility={dataGridTableColumnsVisibility}
        leadingControlColumns={leadingControlColumns}
        data-test-subj="docTable"
        pagination={pagination}
        renderCellValue={renderCellValue}
        rowCount={rowCount}
        sorting={sorting}
        toolbarVisibility={isToolbarVisible ? toolbarVisibility : false}
        rowHeightsOptions={rowHeightsOptions}
      />
      </div>
    ),
    [
      dataGridTableColumns,
      dataGridTableColumnsVisibility,
      leadingControlColumns,
      pagination,
      renderCellValue,
      rowCount,
      sorting,
      isToolbarVisible,
      rowHeightsOptions,
    ]
  );

But this method is not working. Full screen won't trigger re-render even though wrapper

changed, therefore useEffect can't capture the updates.

Check Viewport Dimensions:

Check the dimensions change to see if it matches those of the full screen. This isn't an ideal solution since other factors might cause the window to resize to the screen dimensions.

Extend OuiDataGrid

Since we have flexibility with the codebase, extending or wrapping the OuiDataGrid component to emit custom events or callbacks when the fullscreen state changes might be an ideal solution but this needs more investigation and design.

Conclusion

Ideally we want a way to listen to or detect when the grid goes into fullscreen mode. which might need OuiDataGrid expose an isFullScreen prop or similar API to notify its parent.

In Discover 2.0, we will just disable Full screen util the issue is evaluated and resolved. This is not a regression since Discover legacy is not using OuiDataGrid and has no control on full screen.

ananzh added a commit to ananzh/OpenSearch-Dashboards that referenced this issue Aug 28, 2023
…ight

* disable full screen mode
* customize hide column to delete column
* add move left and move right

Issue Resolve:
opensearch-project#4822
opensearch-project#4823
opensearch-project#4824

Signed-off-by: ananzh <[email protected]>
ananzh added a commit to ananzh/OpenSearch-Dashboards that referenced this issue Aug 30, 2023
…ight

* disable full screen mode
* customize hide column to delete column
* add move left and move right

Issue Resolve:
opensearch-project#4822
opensearch-project#4823
opensearch-project#4824

Signed-off-by: ananzh <[email protected]>
ananzh added a commit to ananzh/OpenSearch-Dashboards that referenced this issue Aug 30, 2023
…ight

* disable full screen mode
* customize hide column to delete column
* add move left and move right

Issue Resolve:
opensearch-project#4822
opensearch-project#4823
opensearch-project#4824

Signed-off-by: ananzh <[email protected]>
ananzh added a commit to ananzh/OpenSearch-Dashboards that referenced this issue Aug 30, 2023
…ight

* disable full screen mode
* customize hide column to delete column
* add move left and move right

Issue Resolve:
opensearch-project#4822
opensearch-project#4823
opensearch-project#4824

Signed-off-by: ananzh <[email protected]>
ananzh added a commit to ananzh/OpenSearch-Dashboards that referenced this issue Aug 30, 2023
…ight

* disable full screen mode
* customize hide column to delete column
* add move left and move right

Issue Resolve:
opensearch-project#4822
opensearch-project#4823
opensearch-project#4824

Signed-off-by: ananzh <[email protected]>
ananzh added a commit that referenced this issue Aug 30, 2023
…ight (#4838)

* disable full screen mode
* customize hide column to delete column
* add move left and move right

Issue Resolve:
#4822
#4823
#4824

Signed-off-by: ananzh <[email protected]>
ashwin-pc pushed a commit to ashwin-pc/OpenSearch-Dashboards that referenced this issue Aug 31, 2023
…ight (opensearch-project#4838)

* disable full screen mode
* customize hide column to delete column
* add move left and move right

Issue Resolve:
opensearch-project#4822
opensearch-project#4823
opensearch-project#4824

Signed-off-by: ananzh <[email protected]>
(cherry picked from commit 2738131)
@ananzh ananzh removed the v2.10.0 label Aug 31, 2023
@ananzh
Copy link
Member Author

ananzh commented Aug 31, 2023

Will come back once OUI solve the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data explorer Issues related to the Data Explorer project discover for discover reinvent
Projects
None yet
Development

No branches or pull requests

1 participant