[New feature] Add flip-view and close buttons to pane-headers #11749

Merged
merged 10 commits into from Oct 15, 2015

Projects

None yet

6 participants

@petetnt
Member
petetnt commented Sep 27, 2015

This new feature improves the Splitscreen UX by adding buttons to panel headers that allow user to quickly flip the current view to another pane. It's not a complete solution, but another alternative to dragging the files over to another panel which can be a bit cumbersome, as evidenced in #11308, #11448, #11345 and others.

When Split View is enabled, arrow buttons are added to the panel that indicate flipping

image

Clicking such button flips the view to the other panel, revealing the last used file in the pane it was flipped from

image

If there's no files open in the new other pane, the button obviously isn't there

image

This of course works in vertical split mode too:

image

The first commit doesn't include unit tests because Jasmine Spec Runner won't work for whatever reason: while I am looking at that issue, any feedback, improvements or questions are welcome.

edit: Build failed because the API rate limit exceeded

edit: 2
The panes now contain a close buttons too as similar to what @zaggino suggested.

image

Clicking the x closes the file, or prompts for confirmation if a file is dirty. If pane contains no files, the close button collapses the views by setting the layout scheme to 1x1

@zaggino
Member
zaggino commented Sep 27, 2015

Love it, gave me an idea -> X mark in the corner to close split view quickly (one click instead of two), would solve my #11746 problem.

@le717 le717 commented on an outdated diff Sep 28, 2015
src/view/Pane.js
@@ -159,17 +159,33 @@ define(function (require, exports, module) {
EventDispatcher = require("utils/EventDispatcher"),
FileSystem = require("filesystem/FileSystem"),
InMemoryFile = require("document/InMemoryFile"),
+ //WorkingSetView = require("project/WorkingSetView"),
@le717
le717 Sep 28, 2015 Contributor

Forget to remove a line?

@le717 le717 commented on the diff Sep 28, 2015
src/styles/brackets.less
+
+ &-flipview-btn {
+ position: relative;
+ display: inline-block;
+ top: 0px;
+ padding-top: 4px;
+ padding-right: 4px;
+ padding-bottom: 2px;
+ padding-left: 4px;
+ margin-left: 0;
+ margin-bottom: 0;
+ .sprite-icon(0, 0, 13px, 13px, "images/flip-view-icons.svg");
+ background-origin: content-box;
+ -webkit-transform: translateZ(0); // forces GPU mode for better filter rendering on retina
+ transform: translateZ(0); // future proofing
+ -webkit-filter: drop-shadow(0 1px 0 rgba(0,0,0,0.36));
@le717
le717 Sep 28, 2015 Contributor

filter has a standardized version. It'd probably be best to add that too like you did with transform.

@petetnt
petetnt Sep 28, 2015 Member

Thanks @le717, fixed those 👍

RE: the filter declaration, there are some other components using the plain vendor-prefixed versions of ones that have standardized ones available, might be worth checking out.

@petetnt
Member
petetnt commented Sep 28, 2015

@zaggino yeah that is an great idea to me!

@abose abose added this to the Release 1.6 milestone Sep 28, 2015
@petetnt
Member
petetnt commented Oct 6, 2015

Added an unit test

@petetnt petetnt changed the title from [REVIEW][New feature] Add flip-view buttons to pane-headers to [REVIEW][New feature] Add flip-view and close buttons to pane-headers Oct 11, 2015
@petetnt
Member
petetnt commented Oct 11, 2015

The panes now contain a close buttons too as similar to what @zaggino suggested.

image

Clicking the x closes the file, or prompts for confirmation if a file is dirty. If pane contains no files, the close button collapses the views by setting the layout scheme to 1x1

@zaggino
Member
zaggino commented Oct 12, 2015

few comments:

  1. i like how the close highlights when hovered, can you make the same with the arrows? right now arrow icon doesn't highlight but some weird border around it highlights

  2. the icons doesn't have to be there all the time, maybe they can appear only when I hover over the title area? (i mean the .pane-header element)

  3. when i click close icon on the last file in the right panel, it doesn't close but i have to click again to close the empty panel? <- this actually might be a feature, not a problem, just saying

  4. if i have one file in the left panel and one file in the right panel, when i close the left file and then close the left pane, the right file disappears (it works fine closing the right panel, left file stays open)

@petetnt
Member
petetnt commented Oct 12, 2015

Thanks for the comments @zaggino. I'll look into those asap, esp. 4), not sure what's up with that 🔨 It seems that getCurrentlyViewedFile() returns the file in the active pane and just clicking on the x doesn't active the pane itself -> wrong file closes.

I was putting some behavior for 2) and 3) behind some prefs: hover, always and off for the panel buttons (default being hover) and something like CLOSE_PANE_AFTER_LAST for the panes:

@zaggino
Member
zaggino commented Oct 12, 2015

Ah right, I didn't read the code before testing this so I didn't knew about the various preferences. But show on hover seems to be reasonable default.

@petetnt
Member
petetnt commented Oct 12, 2015

@zaggino You didn't miss anything, the prefs haven't been implemented yet :)

@zaggino
Member
zaggino commented Oct 12, 2015

Ah right, keep the work coming then, it'd be great to see this in 1.6 :)

@petetnt
Member
petetnt commented Oct 12, 2015

Made additions and modifications according to @zaggino 's comments

  • Unified hover styles for the flip view buttons
  • Added preferences for showing the buttons and collapsing the pane after last file is closed
  • Fixed bug where close button would close the wrong file if the panel was not active
@sprintr sprintr and 1 other commented on an outdated diff Oct 12, 2015
src/view/Pane.js
+ * @const
+ * @private
+ */
+ var FIRST_PANE = "first-pane";
+
+ /**
+ * Internal pane id
+ * @const
+ * @private
+ */
+ var SECOND_PANE = "second-pane";
+
+ // Define showPaneHeaderButtons, which controls when to show close and flip-view buttons
+ // on the header. Possible values "hover", "always" and "never"
+ PreferencesManager.definePreference("pane.showPaneHeaderButtons", "string", "hover", {
+ description: Strings.DESCRIPTION_SHOW_PANE_HEADER_BUTTONS
@sprintr
sprintr Oct 12, 2015 Contributor

You seems to have forgotten to add "hover" "always" "never" to values array here so it can be shown in the hints.

@petetnt
petetnt Oct 13, 2015 Member

Thanks @sprintr , fixed that.

@abose
Contributor
abose commented Oct 15, 2015

@petetnt I can't see the close button when i tested this out. Do I need to do something more to enable close butoon?

@petetnt
Member
petetnt commented Oct 15, 2015

@abose good catch, the close button is there but you cannot see it when you are using a light theme such as Brackets default. Didn't test it out on a lighter theme after adding the close button, will fix asap

@petetnt
Member
petetnt commented Oct 15, 2015

@abose It's now fixed 🔨

@abose
Contributor
abose commented Oct 15, 2015

works now 😎
image
When a pane is empty, the flip button seems to be confused.

@petetnt
Member
petetnt commented Oct 15, 2015

@abose fixed that regression (nitpicky CSS inheriting 🚒).

@abose
Contributor
abose commented Oct 15, 2015

Thanks for this feature @petetnt .
All unit/integration tests passing. LGTM.
When this is ok to be merged, Please remove the [review] tag from the title.

@petetnt petetnt changed the title from [REVIEW][New feature] Add flip-view and close buttons to pane-headers to [New feature] Add flip-view and close buttons to pane-headers Oct 15, 2015
@petetnt
Member
petetnt commented Oct 15, 2015

Ready to be merged as far as I am concerned unless others have something else to add.

@abose abose merged commit 40ad8e9 into adobe:master Oct 15, 2015

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abose
Contributor
abose commented Oct 15, 2015

Thanks @zaggino @sprintr @le717 for reviewing.
Merging.

@petetnt petetnt deleted the petetnt:petetnt/flip-view-buttons branch Oct 15, 2015
@petetnt
Member
petetnt commented Oct 15, 2015

🎆 🎉 👍

@zaggino
Member
zaggino commented Oct 15, 2015

cool stuff, nice work @petetnt ;)

@petetnt petetnt referenced this pull request Oct 15, 2015
Merged

Split View (Same Document) #11820

2 of 2 tasks complete
@arctwelve

@petetnt Congrats on the new feature.

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