fix some working set view issues #9134

JeffryBooher · 2014-09-17T00:15:34Z

redmunds · 2014-09-17T01:32:16Z

This is fixed in Working Set as requested in #8973, but I'm seeing "main.js" on both pane headers. Should the name be qualified there, too?

redmunds · 2014-09-17T01:32:59Z

Fix for #9019 looks good.

JeffryBooher · 2014-09-17T04:14:24Z

@redmunds I'll amend the PR with a fix for that. Good Catch!

JeffryBooher · 2014-09-17T07:08:47Z

@redmunds pr amended with showing the relative path of files in the headers when there are duplicates. Also added a fix for #9032

JeffryBooher · 2014-09-17T07:15:41Z

src/project/FileViewController.js

@@ -200,6 +200,9 @@ define(function (require, exports, module) {
            // image file, we get a null doc here but we still want to keep _fileSelectionFocus
            // as PROJECT_MANAGER. Regardless of doc is null or not, call _activatePane
            // to trigger documentSelectionFocusChange event.
+            _fileSelectionFocus = WORKING_SET_VIEW;
+            _activatePane(paneId);


calling _activatePane here after setting _fileSelectionFocus to WORKING_SET_VIEW will trigger a documentSelectionFocusChange event which will tell the working set view to show the file in the selected state which fixes #9032.

this is what the old master did, btw.

Fix for #9032 looks good except for 1 quirk with single select in project tree.

Split View, Open 4 files -- 2 in each working set

In project tree, single-click file that is open in current working set, but not displayed in editor.

Results:
File is selected in project tree. File is displayed in editor. This is same as 0.43

In project tree, single-click file in the other working set that is not displayed in editor.

Results:
File is selected in project tree. File is displayed in editor. This is same as 0.43

In project tree, single-click file in the other working set that is displayed in editor.

Results:
File is selected in working set.

Expected:
File is selected in project tree.

this should be fixed now.

I'm seeing that case 4 is fixed, but case 3 is now broken.

hmmmm. this worked fine for me last night on the mac and it's working fine for me today on Windows.

#4 - single click file in other working set that IS displayed in editor
-> File is selected in project tree.

@JeffryBooher Yes, 4 is now working for me, but 3 is no longer working.

sorry, I was drawn to the "bold". The problem with #3 is the issue I fixed last night which is still working today for me. LMK after syncing to the latest branch jeff/wsv-fixes and we can look at it over connect.

It's fixed. I guess I didn't pull latest change.

JeffryBooher · 2014-09-17T08:35:00Z

src/project/WorkingSetView.js

+     */
+    WorkingSetView.prototype._updateViewState = function () {
+        var paneId = MainViewManager.getActivePaneId();
+        if ((FileViewController.getFileSelectionFocus() === FileViewController.WORKING_SET_VIEW) &&


if the focus is in the project tree then remove the active class so the blue highlight goes away

redmunds · 2014-09-17T20:25:58Z

@JeffryBooher Can you merge in the latest master so I can test #9142 against latest code?

redmunds · 2014-09-17T20:40:10Z

src/project/WorkingSetView.js

@@ -690,6 +706,8 @@ define(function (require, exports, module) {
            }

            this._redraw();
+        } else {
+            this._checkForDuplicatesInWorkingTree();


The suppressRedraw param here is a bit weird. Before this change, if suppressRedraw is true, then method does nothing. Seems like caller should detect that and not even call it?

I'm wondering if _checkForDuplicatesInWorkingTree() needs to be called when suppressRedraw is true?

Looks like we want to ignore suppressRedraw and just check paneId:

if (paneId === this.paneId) { if (!suppressRedraw) { // snip... } } else { this._checkForDuplicatesInWorkingTree(); }

Keep in mind that there are 2 working set views...

The suppress redraw flag is only affecting the one that's being updated which won't check for duplicates when the suppressRedraw flag is true.

This is used in 2 cases:

When an untitled document is being saved.

When a file is saved with a new name.

The pane that isn't affected by the change, however, is free to draw when it needs to with check for duplicates.

The other thing to keep in mind is that when using the suppressRedraw flag, it's just to prevent removing list items when they are being removed from the working set and a new one is being added in a new place. Check for duplicates doesn't remove anything so we would still be able to check for duplicates when suppressRedraw is passed on the affected view.

That's a great explanation that belongs in the code somewhere!

redmunds · 2014-09-17T20:56:49Z

Done with review.

…igorously wrt _suppressRedraw

JeffryBooher · 2014-09-18T06:24:32Z

@redmunds I reverted the change that undid @larz0 styling issue but by using a variable and removed #9142 from the list of issues fixed with this pr. @larz0 do you want to review that change?

redmunds · 2014-09-18T15:26:49Z

src/styles/brackets.less

-            color: @open-working-file-ext-highlight;
-        }
-    }
-}


I don't know if it's related to this change, but I wanted to make this comment in code to start a discussion thread (as opposed to main comments which are no threaded).

This may be as designed or FOL, but this behavior is unexpected:

Open Split View with 1 file in each Working Set
Results: text color of both Working Set files is blue when either is selected as expected.

Single-click on some other file in Project Tree
Results: text color of file in Project Tree is blue. Text color of file in other Working Set is blue. Expected.

Select Working Set file in other pane as file in Project Tree

Results: text of file in current Working Set is the only file in blue.
Expected: text of file displayed in other pane (i.e. file in Project Tree) is blue

WRT #3, I think this is correct

Open 2 files (editor.js in one pane, mainviewmanager.js in another)

Click on a file that isn't open anywhere (pane.js)
--> pane.js and editor.js are both blue and both visible in panes. mvm is hidden.

Click editor.js in file tree
--> editor.js is the only file that is blue (it's blue in both the project tree and workingsetview)

This is because pane.js is open in the other pane which isn't in the working set.
I can see how that may lead to some confusion as pane.js isn't blue anywhere because file tree doesn't allow for more than one selected item.

I wonder if @dangoor's new file tree would allow for this and if it would even make sense. I guess it could be blue and not "selected" but when we allow for the same document to be open in 2 different panes would that even make sense?

fix a couple of working set view issues

4e2b40c

JeffryBooher added the PICK ME UP label Sep 17, 2014

redmunds self-assigned this Sep 17, 2014

redmunds removed the PICK ME UP label Sep 17, 2014

JeffryBooher added 3 commits September 16, 2014 23:36

fix 2 more issues

6b511cd

fix rte

2211499

merge with master

9ee3a2a

JeffryBooher changed the title ~~fix two working set view issues~~ fix some working set view issues Sep 17, 2014

JeffryBooher reviewed Sep 17, 2014
View reviewed changes

fix selector issue with selected item

9433ad8

JeffryBooher reviewed Sep 17, 2014
View reviewed changes

restore the order

eba535c

Merge branch 'master' into jeff/wsv-fixes

fb2b075

redmunds reviewed Sep 17, 2014
View reviewed changes

redmunds mentioned this pull request Sep 17, 2014

Working set item remains blue when selection highlight moves to folder #9142

Closed

JeffryBooher added 2 commits September 17, 2014 14:30

rtc: document _suppresRedraw flag and qualify affected working more r…

4df265e

…igorously wrt _suppressRedraw

restore the blues

eefe6b3

fix tree selection issue

2ecef07

redmunds reviewed Sep 18, 2014
View reviewed changes

Merge branch 'master' into jeff/wsv-fixes

48f6486

JeffryBooher closed this Sep 18, 2014

JeffryBooher mentioned this pull request Sep 18, 2014

Fix a few working set related issues #9160

Merged

JeffryBooher deleted the jeff/wsv-fixes branch October 7, 2014 17:47

core-ai-bot mentioned this pull request Aug 30, 2021

[CLOSED] fix some working set view issues brackets-archive/bracketsIssues#8161

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix some working set view issues #9134

fix some working set view issues #9134

JeffryBooher commented Sep 17, 2014

redmunds commented Sep 17, 2014

redmunds commented Sep 17, 2014

JeffryBooher commented Sep 17, 2014

JeffryBooher commented Sep 17, 2014

JeffryBooher Sep 17, 2014

JeffryBooher Sep 17, 2014

redmunds Sep 17, 2014

JeffryBooher Sep 18, 2014

redmunds Sep 18, 2014

JeffryBooher Sep 18, 2014

redmunds Sep 18, 2014

JeffryBooher Sep 18, 2014

redmunds Sep 18, 2014

JeffryBooher Sep 17, 2014

redmunds commented Sep 17, 2014

redmunds Sep 17, 2014

redmunds Sep 17, 2014

JeffryBooher Sep 17, 2014

redmunds Sep 17, 2014

redmunds commented Sep 17, 2014

JeffryBooher commented Sep 18, 2014

redmunds Sep 18, 2014

JeffryBooher Sep 18, 2014

fix some working set view issues #9134

fix some working set view issues #9134

Conversation

JeffryBooher commented Sep 17, 2014

redmunds commented Sep 17, 2014

redmunds commented Sep 17, 2014

JeffryBooher commented Sep 17, 2014

JeffryBooher commented Sep 17, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

redmunds commented Sep 17, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

redmunds commented Sep 17, 2014

JeffryBooher commented Sep 18, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment