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

New Dialog Component #275

Merged

Conversation

jorgenherje
Copy link
Collaborator

@jorgenherje jorgenherje commented Oct 13, 2022

New draggable dialog component for usage in Webviz.

Dialog component draggable, with width and position adjustment when window width is resized. Functionality for handling multiple dialogs open at the same time, by controlling css for currently active dialog. This will make currently active dialog on top and with highlighting. The dialogs can be set modal - resulting in a clickable backdrop with adjusted opacity. When backdrop is clicked, the modal dialog is closed.

All dialogs are placed within a div as child of <body> named WebvizDialog__root. Thereby z-index can be controlled and the content of the WebvizDialog__root will have maximum one active dialog - controlled by the WebvizDialog--active class name.


  • Add usage of new WebvizDialog for settings in WebvizViewElement

Closes: #261

jorgenherje and others added 30 commits September 30, 2022 09:46
- Improve resize logic
- Remove unnecessary components
- Improve and move util to geometry.ts
- Improve css naming
- Add handle of blur
- Improve verification of clientX and clientY values < 0.
- Improve resizing when size is increased and size on right < rightPadding
This was necessary in order to make `dash-generate-components` work.
- Only perform window resize event when dialog is open.
- Detect if dialog is outside window when opening
- Adjust size according to available space when opening dialog
Did not adjust width on first open
Utilize mutationObserver to detect when dialog is open and adjust
position and width accordingly.
Only need for keyup event listener
- stop propagation of native mousedown and touchstart for Close button
using addEventListener.
- Could not handle with onMouseDown and onTouchStart
Added new `Renderer` component to move `createPortal` usage out of main component in order to make Dash's `generate-components` command work.
- Renderer was initially returning <div></div>, this initial
mutationObserver call was not performed. Resulting in incorrect
initial width and position.
- Add scrollArea for content when height of dialog > half window size
when opening
Gave issues with initial position and size when initial open state = true.
Thereby it was no mutation of attribute and thereby no adjustment
of dialog position and size.
- Utilize Size object from geometry
- Fix incorrect code for activating ScrollArea
Copy link
Collaborator

@rubenthoms rubenthoms left a comment

Choose a reason for hiding this comment

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

LGTM - some comments/suggestions.

react/src/lib/components/Backdrop/Backdrop.tsx Outdated Show resolved Hide resolved
react/src/lib/components/Backdrop/backdrop.css Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/WebvizDialog.tsx Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/WebvizDialog.tsx Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/WebvizDialog.tsx Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/WebvizDialog.tsx Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/WebvizDialog.tsx Outdated Show resolved Hide resolved
- Remove isMovedOutsideWindow state and check if dialog is outside the
previous window directly in the window resize handler.
- Minor bug fix for window width increase when actual dialog margin
right is less than margin.right. Made dialog left jump with margin.right
- Did not work correctly when content was sized to trigger scrollArea
…nents

- Add heightOwner prop to control height of dialog - no height owner
implies that height is adjusted according to content
- Do not use ScrollArea if heightOwner is undefined - height adjusts to
content
- Refactor dialog content and dialog actions into separate sub-components
- Use min/max width
- Remove min/max height usage
- Prevent automatic calculation of width/height as content must be
more controlled - gave resize issues and initializing issues.
- Control of z-index to ensure correct dialog "on top" at all time
For correct placeholder className and z-index
- Add dialog to easier show correct behavior of z-index control with
three non-modal dialogs.
@jorgenherje jorgenherje marked this pull request as ready for review November 3, 2022 09:45
Copy link
Collaborator

@rubenthoms rubenthoms left a comment

Choose a reason for hiding this comment

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

LGTM 👍 Some comments and considerations below.

react/src/lib/components/WebvizDialog/webviz-dialog.css Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/webviz-dialog.css Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/WebvizDialog.tsx Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/WebvizDialog.tsx Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/WebvizDialog.tsx Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/WebvizDialog.tsx Outdated Show resolved Hide resolved
react/src/lib/components/WebvizDialog/WebvizDialog.tsx Outdated Show resolved Hide resolved
Copy link
Collaborator

@rubenthoms rubenthoms left a comment

Choose a reason for hiding this comment

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

One line that could be changed, otherwise LGTM, good work 👍

@jorgenherje jorgenherje merged commit f5311c4 into equinor:master Nov 15, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Done 🏁
Development

Successfully merging this pull request may close these issues.

Create new Dialog component
2 participants