Skip to content

Add keybindings for resizing frames#754

Closed
djbarnwal wants to merge 2 commits intoiodide-project:iframe-2from
djbarnwal:iframe2-fix
Closed

Add keybindings for resizing frames#754
djbarnwal wants to merge 2 commits intoiodide-project:iframe-2from
djbarnwal:iframe2-fix

Conversation

@djbarnwal
Copy link
Copy Markdown
Contributor

@djbarnwal djbarnwal commented Aug 14, 2018

This implements Ctrl+left and Ctrl+right keybindings which help you resize the two frames in steps of 0%, 33%, 50%, 66% and 100% .

Closes Issue #726

Copy link
Copy Markdown
Contributor

@bcolloran bcolloran 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 overall, a few small changes in the comments.

(and of course merge master in)

}}
>
<div id="cells">
<div style={this.props.cellsClass} id="cells">
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

cellsClass shoulc be cellsStyle

handleClasses={{ right: 'resizer' }}
bounds="window"
minWidth={300}
minWidth={0}
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

allowing minWidth=0 allows some weird behavior when manually pulling the divider to the extreme right. this is not a big deal, but maybe we should pass in minWidth with props so that we can set it to 300 when we're not in the zero width condition, and we can set it to zero in the zero width condition.

Comment thread src/editor-state-prototypes.js Outdated
// for editorWidth calc. We're using this
// because re-resizable doesn't intelligently deal w/ calc(100% - 20px) or whatever.
const DEFAULT_EDITOR_WIDTH = Math.round(document.documentElement.clientWidth / 2)
const SCREEN_WIDTH = document.documentElement.clientWidth
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

SCREEN_WIDTH needs to update with window resize

// )

export const paneSizes = [0, 0.33, 0.5, 0.66, 1].map(x => Math.round(x * SCREEN_WIDTH))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

export the const with [0, 0.33, 0.5, 0.66, 1], and compute the actual pixel widths in the reducer (since SCREEN_WIDTH will need to update on resize)

if (width < paneSizes[1]) width = paneSizes[1]
else if (width < paneSizes[2]) width = paneSizes[2]
else if (width < paneSizes[3]) width = paneSizes[3]
else width = paneSizes[4]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

calculate pixel vals of paneSizes here


case 'DECREASE_EDITOR_WIDTH': {
let width = state.editorWidth
if (width > paneSizes[3]) width = paneSizes[3]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

calculate pixel vals of paneSizes here

@bcolloran bcolloran added this to the 0.6a - iframe2 milestone Aug 15, 2018
@bcolloran
Copy link
Copy Markdown
Contributor

@djbarnwal let me know when you are ready for another review on this one. i see you still need to merge master, so i'm not sure if you have other things planned as well.

@bcolloran bcolloran closed this Aug 23, 2018
@bcolloran
Copy link
Copy Markdown
Contributor

@djbarnwal oops, can you push this branch again @djbarnwal

@djbarnwal djbarnwal deleted the iframe2-fix branch September 5, 2018 16:54
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.

2 participants