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

Update droplet dropdowns to extend outside the coding workspace (reintroduces reverted PR) #29849

Merged
merged 9 commits into from
Jul 24, 2019

Conversation

jmkulwik
Copy link
Contributor

@jmkulwik jmkulwik commented Jul 23, 2019

Reintroduce this PR: #29588 which was reverted because the app display was being cut off along the edges. See https://eyes.applitools.com/app/test-results/00000251838817645025/?accountId=mG4DoRfv0ntEgI3XIYBulrKxlZqrHtGwbP3V103ojvLFU110 for details.

In the past, droplet dropdowns were cut off when they left the app workspace. Now, dropdowns are visible outside the workspace.

Relates to this PR in the droplet repo: droplet-editor/droplet#211

OLD:
image

NEW:
image

@jmkulwik jmkulwik changed the title Fix blockly dropdown take3 Update droplet dropdowns to extend outside the coding workspace (reintroduces reverted PR) Jul 23, 2019
@@ -84,7 +84,7 @@ class CodeWorkspace extends React.Component {
if (textbox.style.bottom) {
$(textbox).animate(
{bottom: debuggerHeight},
{step: utils.fireResizeEvent}
{done: utils.fireResizeEvent}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixes a long-standing issue where the code workspace would resize to the wrong size. This is especially apparent on computers that are close to their maximum ram when running applab. This was fixed as part of this PR because the droplet changes exacerbated the issue. I checked with @ryansloan, and he agrees that the new UI is preferred.

OLD
workspaceRendering

NEW
workspaceRenderingNew

Copy link
Contributor

Choose a reason for hiding this comment

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

Awesome!

@@ -44,13 +44,17 @@
opacity: 1;
}

// When opening the debug console with the ellipsis, the codeTextbox sometimes
// renders over the debug console. Setting the z-index to 2 is a work-around for
// this issue. See JsDebugger.jsx.
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for capturing this issue. Even though it is harder, I think we should do the 'hard' solution that you propose in that ticket.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree. The "easy" option might just be a stop-gap while we work on the "hard" option.

@@ -98,8 +98,7 @@ const styles = {
borderLeftStyle: 'none',
borderTopWidth: 1,
borderTopStyle: 'solid',
borderTopColor: '#ddd',
overflow: 'hidden'
Copy link
Contributor

Choose a reason for hiding this comment

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

My only concern here is unexpected side effects with Blockly. I assume you have already verified that droplet is ok with this change. If you've tested various Blockly levels as well, I'm ok.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for noting that. I went ahead and spot-checked a couple blockly levels, and they look good.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants