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 jumpy notebook behavior #1853

Closed
wants to merge 3 commits into from
Closed

Fix jumpy notebook behavior #1853

wants to merge 3 commits into from

Conversation

mcelrath
Copy link

@mcelrath mcelrath commented Jun 4, 2012

Menu: the menu items need a 1px border so that when one is added by jQuery, the menu item doesn't shift by 1px. (ui-menu and ui-menu-item classes)

Cell focus: The jQuery class .ui-widget-content draws the border on the focused cell, but the unfocused cell doesn't have a border, causing 1px jumping. The solution is to add a border for it, but then this will have higher specificity than the jQuery class. So I also added a redefinition of the focused cell border with div.ui-widget-content, which has higher specificity. A selected cell was additionally getting an outline from jQuery when :active. I also disabled this by setting 'outline: none', to prevent flashing of the border when a cell is clicked on.

Cell focus scrolling: The CodeMirror focus() call causes the window to scroll to the CodeMirror input cell. I removed this call from CodeCell.prototype.select() and RawCell.prototype.select(). Additionally, the CodeMirror refresh() call is only required if the input cell changes and must be redrawn. This doesn't happen on focus, it happens on edit, and refresh() is properly called in TextCell.prototype.edit(), so isn't required in select(). I removed these superfluous refresh() calls too.

mcelrath added 3 commits June 4, 2012 14:55
cell's input field when output field is clicked.
… to cell's input field when output field is clicked; enable tab indenting and paren matching.
Conflicts:
	IPython/frontend/html/notebook/static/js/codecell.js
@mcelrath mcelrath closed this Jun 4, 2012
@fperez
Copy link
Member

fperez commented Jun 4, 2012

Just curious, why was this closed?

@mcelrath
Copy link
Author

mcelrath commented Jun 4, 2012

Fernando Perez [reply@reply.github.com] wrote:

Just curious, why was this closed?

Because I'm just learning how to use git, and the commit contained things it
shouldn't have. (Unrelated syntax error) I'll re-commit in a few minutes.

So let me ask...what is the appropriate granularity for commits and pull
requests? I have a few things that are more-or-less one liners:

  1. Enable paren matching
  2. Enable tab indentation (highlight text, tab/shift-tab indent/outdent --
    important for python!)
  3. 1px/border jumping in cells, menu
  4. click causing refocus and scrolling of output

#1 and #2 are just one-line configuration options to CodeMirror. The others
similarly are very small.

Should I make 4 commits and 4 pull requests?

Cheers, Bob McElrath

"The individual has always had to struggle to keep from being overwhelmed by
the tribe. If you try it, you will be lonely often, and sometimes frightened.
But no price is too high to pay for the privilege of owning yourself."
-- Friedrich Nietzsche

@fperez
Copy link
Member

fperez commented Jun 5, 2012

We don't have any hard rules on commit granularity, but you should keep to the usual guideline of making a commit a reasonably localized (conceptually) operation. A single commit may touch several files (sometimes many), but it should 'be one thing'. Basically if a commit can't be well described by the first line summary plus at most a handful of lines of detail, it's probably too big.

As for pull requests, they should be strongly confined to soiving one problem only: a bug fix, implementing a feature, etc. Now, sometimes 'one problem' may be absolutely huge, but such monsters are best avoided (in that case it really was 'one thing', just that it was something that touched pretty much all of IPython).

And conceptually, the PR really should have a strong unity. Just like a commit should be explainable in a concise form, the PR should also be well described by the summary box github offers. If you find that you're describing a bunch of loosely related things, that may be a sign to break up the PR into several.

Keep in mind: short, simple PRs are really easy to review, so if you have to, err on the side of more smaller ones rather than fewer bigger ones. I (or someone else) may have 10 minutes to review/merge one or two small ones, but if I see something huge I'll keep postpoining it until I have a few hours free.

Since multiple people can and do review, by making them more digestible you dramatically raise the chance they'll get processed rapidly.

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.

None yet

2 participants