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

fix(overlay): adjust max heights to always fit viewport #2915

Merged
merged 1 commit into from
Apr 19, 2022

Conversation

smbea
Copy link
Contributor

@smbea smbea commented Apr 11, 2022

Closes #2882

Screen.Recording.2022-04-12.at.14.29.50.mov

@bpmn-io-tasks bpmn-io-tasks bot added the needs review Review pending label Apr 11, 2022
@@ -27,7 +27,6 @@ export function ReportFeedbackOverlay(props) {
<Overlay
anchor={ props.anchor }
onClose={ props.onClose }
maxHeight="calc(100vh - 250px)"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Explanation: this was causing the the report feedback overlay to shorten and have scroll, even though there was plenty of space still. The height of this overlay is a lot smaller than MIN_HEIGHT of the Modeler, meaning we don't need to set this.

@smbea smbea requested review from a team, philippfromme, MaxTru and andreasgeier and removed request for a team April 11, 2022 09:38
Copy link

@andreasgeier andreasgeier left a comment

Choose a reason for hiding this comment

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

Please ensure that the scrolling container takes the full width of the overlay so that the scrollbar is on the edge and not overlapping form elements.

@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Apr 11, 2022
@andreasgeier
Copy link

Expected:

Screen.Recording.2022-04-11.at.17.30.39.mov

@smbea
Copy link
Contributor Author

smbea commented Apr 12, 2022

@andreasgeier I updated the screen recording accordingly

Copy link

@andreasgeier andreasgeier left a comment

Choose a reason for hiding this comment

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

Works as expected 👍

@smbea smbea added needs review Review pending and removed in progress Currently worked on labels Apr 19, 2022
Copy link
Contributor

@pinussilvestrus pinussilvestrus left a comment

Choose a reason for hiding this comment

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

Looks good so far 👍

client/src/shared/ui/overlay/Overlay.less Show resolved Hide resolved
client/src/shared/ui/overlay/Overlay.less Outdated Show resolved Hide resolved
@bpmn-io-tasks bpmn-io-tasks bot added in progress Currently worked on and removed needs review Review pending labels Apr 19, 2022
Copy link
Contributor

@pinussilvestrus pinussilvestrus left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@smbea smbea merged commit dc0b099 into develop Apr 19, 2022
@smbea smbea deleted the 2882-overlay-overflow branch April 19, 2022 09:27
@bpmn-io-tasks bpmn-io-tasks bot removed the in progress Currently worked on label Apr 19, 2022
@Skaiir Skaiir mentioned this pull request Jul 8, 2022
40 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Cloud deploy modal is cut off from the window.
3 participants