Skip to content
This repository was archived by the owner on Sep 1, 2022. It is now read-only.

Added bootstrap-tags 1.1 #32

Merged
merged 3 commits into from
Oct 15, 2014
Merged

Added bootstrap-tags 1.1 #32

merged 3 commits into from
Oct 15, 2014

Conversation

jdfreder
Copy link
Contributor

@jdfreder jdfreder commented Oct 7, 2014

No description provided.

@jdfreder
Copy link
Contributor Author

jdfreder commented Oct 7, 2014

ipython/ipython#6638

@jdfreder jdfreder mentioned this pull request Oct 7, 2014
@@ -0,0 +1,7 @@
guard 'rake', :task => 'compile' do
Copy link
Member

Choose a reason for hiding this comment

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

exclude this file via .gitignore

@jdfreder
Copy link
Contributor Author

jdfreder commented Oct 7, 2014

@minrk comments addressed and rebased :)

@jdfreder
Copy link
Contributor Author

jdfreder commented Oct 7, 2014

Unrebased till @Carreau CM4 PR gets merged into ipython\ipython master.

@jdfreder jdfreder force-pushed the bootstrap-tags branch 2 times, most recently from ff801d2 to 1bd8717 Compare October 7, 2014 17:42
@jhamrick
Copy link
Member

jhamrick commented Oct 7, 2014

  • After you click the filter button, it expands; if you click it again, it collapses, and then can't be expaned again.
  • If you start typing in the cell tags area and then click out of the area (or press tab), the tag doesn't get rendered. My expectation is that anything typed into the tags area should become a tag if focus leaves the tags area?

image

  • Is the idea with filtering to be just visual, or should it affect the cells that are run, too? Currently, if you filter out some cells and click "run all", all cells are still run (even hidden ones). Is that intentional?
  • If the cursor is in the cell tags area and you press up/down, it does not cause the next/previous cell to be selected (which is what I would have expected). It seems the only way to get out of the tags area via the keyboard is to press "tab", which will move you into the text of the cell if it's a code cell, or to the tags area of the next cell. So if you wanted to go to a previous cell, you would have to do n tabs, escape, then n ups = 2n + 1 key presses, or use the mouse.

@jdfreder
Copy link
Contributor Author

jdfreder commented Oct 7, 2014

@jhamrick see my response in ipython/ipython#6638

@ellisonbg
Copy link
Contributor

So cool, some feedback:

  • Rendered markdown and heading cells lack the padding inside the selected border. Bug in master also, but I don't want to forget.
  • Looks like the celltoolbar background color is different for text and code cells?
  • Cells with no tags are no affected by the filter. How should they be treated?
  • We should at least register an ESC handler for the tag text area so users can get back to command mode. But that should automatically happen if you are using the keyboard_manager stuff?
  • Expression like tag1 and (not tag2) are not working, but tag1 and not tag2 does.
  • We should probably save the current filter in notebook metadata and reapply upon a page load.
  • Let's remove the glowing blue border from the filter text area when selected.

@jdfreder
Copy link
Contributor Author

@ellisonbg I will respond in the main PR- ipython/ipython#6638

@jdfreder
Copy link
Contributor Author

rebased

@@ -6,6 +6,7 @@
"bootstrap": "components/bootstrap#~3.1",
"bootstrap-tour": "0.9.0",
"codemirror": "~4.6.0",
"bootstrap-tags": "1.1",
Copy link
Member

Choose a reason for hiding this comment

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

can you add this just below "bootstrap" to preserve alphabetical order?

@minrk
Copy link
Member

minrk commented Oct 15, 2014

looks like it still needs a rebase. Can you put bootstrap-tags above bootstrap-tour? If you do that, I'll go ahead and merge when it's green.

@jdfreder
Copy link
Contributor Author

@minrk alphabetized and rebased!

@minrk minrk changed the title Added bootstrap-tags master. Added bootstrap-tags 1.1 Oct 15, 2014
minrk added a commit that referenced this pull request Oct 15, 2014
@minrk minrk merged commit a75ece4 into jupyter:master Oct 15, 2014
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants