Skip to content

Commit 858bf57

Browse files
committed
revert: move back/forward, show-in-tree, search buttons to mainNavBar
User feedback indicated the old position (top-right of the sidebar nav) was better than the relocated positions inside the central control bar and the sidebar project-files-header. Restore the four divs in #mainNavBarRight with their original sprite-icon classes, drop the CCB .ccb-group-nav block and the sidebar-hover #show-in-file-tree button, and prune the CSS overrides that were added to host the buttons in the CCB. NavigationProvider wiring is unchanged — it still attaches to these IDs. Tests: - Move #showInfileTree and #searchNav click-dispatch tests to the NavigationAndHistory integ suite. - Add a scroll-to-selected test in SidebarTabs-integ-test for handleShowInTree's post-showInTree ViewUtils.scrollElementIntoView (previously uncovered). - Drop the obsolete CCB-side button tests.
1 parent 26317ae commit 858bf57

7 files changed

Lines changed: 96 additions & 98 deletions

File tree

src/extensionsIntegrated/CollapseFolders/main.js

Lines changed: 11 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,14 @@
1818
*
1919
*/
2020

21-
/* Displays sidebar-hover action buttons: "show in file tree" (binoculars) and
22-
* "collapse all folders" (stacked chevrons). Both appear on sidebar hover so the
23-
* sidebar stays visually quiet when the user isn't interacting with it. */
24-
/* Styling for both buttons is done in `../../styles/Extn-CollapseFolders.less` */
21+
/* Displays a Collapse button in the sidebar area */
22+
/* when the button gets clicked, it closes all the directories recursively that are opened */
23+
/* Styling for the button is done in `../../styles/Extn-CollapseFolders.less` */
2524
define(function (require, exports, module) {
2625
const AppInit = require("utils/AppInit");
27-
const CommandManager = require("command/CommandManager");
28-
const Commands = require("command/Commands");
2926
const ProjectManager = require("project/ProjectManager");
3027
const Strings = require("strings");
3128

32-
const SHOW_IN_TREE_SVG = '<svg class="show-in-tree-icon" viewBox="0 0 16 16" fill="none" xmlns="http://www.w3.org/2000/svg" aria-hidden="true">' +
33-
'<path fill="currentColor" d="M4.5 1A1.5 1.5 0 0 0 3 2.5V3h4v-.5A1.5 1.5 0 0 0 5.5 1h-1zM7 4v1h2V4h4v.882a.5.5 0 0 0 .276.447l.895.447A1.5 1.5 0 0 1 15 7.118V13H9v-1.5a.5.5 0 0 1 .146-.354l.854-.853V9.5a.5.5 0 0 0-.5-.5h-3a.5.5 0 0 0-.5.5v.793l.854.853A.5.5 0 0 1 7 11.5V13H1V7.118a1.5 1.5 0 0 1 .83-1.342l.894-.447A.5.5 0 0 0 3 4.882V4h4zM1 14v.5A1.5 1.5 0 0 0 2.5 16h3A1.5 1.5 0 0 0 7 14.5V14H1zm8 0v.5a1.5 1.5 0 0 0 1.5 1.5h3a1.5 1.5 0 0 0 1.5-1.5V14H9zm4-11H9v-.5A1.5 1.5 0 0 1 10.5 1h1A1.5 1.5 0 0 1 13 2.5V3z"/>' +
34-
'</svg>';
35-
3629
/**
3730
* This is the main function that handles the closing of all the directories
3831
*/
@@ -59,37 +52,30 @@ define(function (require, exports, module) {
5952
}
6053
}
6154

62-
function _handleShowInTreeClick() {
63-
CommandManager.execute(Commands.NAVIGATE_SHOW_IN_FILE_TREE);
64-
}
65-
6655
/**
67-
* Append the sidebar hover actions: a "Show in File Tree" binoculars button
68-
* followed by the "Collapse All" chevron button. Both live in
69-
* #project-files-header and become visible only on #sidebar:hover.
56+
* This function is responsible to create the 'Collapse All' button
57+
* and append it to the sidebar area on the project-files-header
7058
*/
71-
function createSidebarHoverButtons() {
59+
function createCollapseButton() {
7260
const $projectFilesHeader = $("#project-files-header");
61+
// make sure that we were able to get the project-files-header DOM element
7362
if ($projectFilesHeader.length === 0) {
7463
return;
7564
}
7665

77-
const $showInTreeBtn = $('<div id="show-in-file-tree" class="btn-alt-quiet" title="' +
78-
Strings.CMD_SHOW_IN_TREE + '">' + SHOW_IN_TREE_SVG + '</div>');
79-
$showInTreeBtn.on("click", _handleShowInTreeClick);
80-
$projectFilesHeader.append($showInTreeBtn);
81-
66+
// create the collapse btn
8267
const $collapseBtn = $(`
8368
<div id="collapse-folders" class="btn-alt-quiet" title="${Strings.COLLAPSE_ALL_FOLDERS}">
8469
<i class="fa-solid fa-chevron-down collapse-icon" aria-hidden="true"></i>
8570
<i class="fa-solid fa-chevron-up collapse-icon" aria-hidden="true"></i>
8671
</div>
8772
`);
73+
8874
$collapseBtn.on("click", handleCollapseBtnClick);
89-
$projectFilesHeader.append($collapseBtn);
75+
$projectFilesHeader.append($collapseBtn); // append the btn to the project-files-header
9076
}
9177

9278
AppInit.appReady(function () {
93-
createSidebarHoverButtons();
79+
createCollapseButton();
9480
});
9581
});

src/index.html

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -914,6 +914,10 @@
914914
<a id="phcode-io-main-nav" href="https://phcode.io" target="_blank" rel="noopener">phcode.io</a>
915915
</div>
916916
<div id="mainNavBarRight">
917+
<div id="navBackButton" class="nav-back-btn btn-alt-quiet"></div>
918+
<div id="navForwardButton" class="nav-forward-btn btn-alt-quiet"></div>
919+
<div id="showInfileTree" class="show-in-file-tree-btn btn-alt-quiet"></div>
920+
<div id="searchNav" class="search-nav-btn btn-alt-quiet"></div>
917921
<div class="working-set-splitview-btn btn-alt-quiet"></div>
918922
</div>
919923
</div>
@@ -965,12 +969,6 @@
965969
<i class="fa-solid fa-floppy-disk"></i>
966970
</a>
967971
</div>
968-
<div class="ccb-divider"></div>
969-
<div class="ccb-group ccb-group-nav">
970-
<a href="#" id="searchNav" class="ccb-btn"><i class="fa-solid fa-magnifying-glass"></i></a>
971-
<a href="#" id="navBackButton" class="ccb-btn"><i class="fa-solid fa-arrow-left"></i></a>
972-
<a href="#" id="navForwardButton" class="ccb-btn"><i class="fa-solid fa-arrow-right"></i></a>
973-
</div>
974972
</div>
975973

976974
<!--

src/styles/CentralControlBar.less

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -107,25 +107,6 @@
107107
}
108108
}
109109

110-
/* Nav buttons: suppress legacy sprite styles (NavigationProvider toggles enabled/disabled class) */
111-
#searchNav,
112-
#navBackButton,
113-
#navForwardButton {
114-
background: transparent !important;
115-
top: auto !important;
116-
padding: 0 !important;
117-
width: auto !important;
118-
height: 28px !important;
119-
content: normal !important;
120-
opacity: 1;
121-
}
122-
123-
#navBackButton.nav-back-btn-disabled,
124-
#navForwardButton.nav-forward-btn-disabled {
125-
opacity: 0.35;
126-
pointer-events: none;
127-
}
128-
129110
}
130111

131112
/* Editor collapse: actual layout handled in JS via .content width/visibility */
Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,42 +1,25 @@
1-
#collapse-folders,
2-
#show-in-file-tree {
1+
#collapse-folders {
32
display: flex;
3+
flex-direction: column;
44
align-items: center;
55
justify-content: center;
66
padding: 0.2em 0.65em;
77
margin-top: 0.1em;
88
position: absolute !important;
9+
right: 0;
910
opacity: 0;
1011
visibility: hidden;
1112
transition:
1213
opacity 0.2s ease-in-out,
1314
visibility 0.2s ease-in-out;
14-
}
15-
16-
#collapse-folders {
17-
flex-direction: column;
18-
right: 0;
1915

2016
.collapse-icon {
2117
font-size: 0.5em;
2218
line-height: 1;
2319
}
2420
}
2521

26-
#show-in-file-tree {
27-
// Sit to the left of #collapse-folders.
28-
right: 24px;
29-
30-
.show-in-tree-icon {
31-
// Sized to match the combined stacked-chevron glyph of #collapse-folders.
32-
width: 11px;
33-
height: 11px;
34-
pointer-events: none;
35-
}
36-
}
37-
38-
#sidebar:hover #collapse-folders,
39-
#sidebar:hover #show-in-file-tree {
22+
#sidebar:hover #collapse-folders {
4023
opacity: 1;
4124
visibility: visible;
4225
}

test/spec/CentralControlBar-integ-test.js

Lines changed: 0 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -246,14 +246,6 @@ define(function (require, exports, module) {
246246
expect(executed).toContain(Commands.FILE_SAVE);
247247
});
248248

249-
it("should trigger CMD_FIND_IN_FILES when the search button is clicked", function () {
250-
const executed = recordCommands(function () {
251-
_$("#searchNav").trigger("click");
252-
});
253-
// searchNav routes through NavigationProvider._findInFiles which dispatches CMD_FIND_IN_FILES.
254-
expect(executed).toContain(Commands.CMD_FIND_IN_FILES);
255-
});
256-
257249
it("should execute VIEW_HIDE_SIDEBAR when #ccbSidebarToggleBtn is clicked, and the sidebar's visibility actually flips", async function () {
258250
// Measure the effect: sidebar is visible → click → sidebar hides.
259251
expect(SidebarView.isVisible()).toBe(true);
@@ -300,25 +292,6 @@ define(function (require, exports, module) {
300292
});
301293
});
302294

303-
describe("2a. #show-in-file-tree button in sidebar", function () {
304-
305-
it("should render #show-in-file-tree inside #project-files-header, before #collapse-folders, with the localized title", function () {
306-
const $btn = _$("#show-in-file-tree");
307-
expect($btn.length).toBe(1);
308-
expect($btn.parent().attr("id")).toBe("project-files-header");
309-
// #collapse-folders sits after #show-in-file-tree in DOM order.
310-
expect($btn.nextAll("#collapse-folders").length).toBe(1);
311-
expect($btn.attr("title")).toBe(Strings.CMD_SHOW_IN_TREE);
312-
});
313-
314-
it("should execute NAVIGATE_SHOW_IN_FILE_TREE when #show-in-file-tree is clicked", function () {
315-
const executed = recordCommands(function () {
316-
_$("#show-in-file-tree").trigger("click");
317-
});
318-
expect(executed).toContain(Commands.NAVIGATE_SHOW_IN_FILE_TREE);
319-
});
320-
});
321-
322295
describe("3. Toggle Design Mode command", function () {
323296

324297
it("should execute VIEW_TOGGLE_DESIGN_MODE and flip isInDesignMode() from false to true and back", async function () {

test/spec/Extn-NavigationAndHistory-integ-test.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,37 @@ define(function (require, exports, module) {
344344
await _expectNavButton(false, true, "nav forward disabled due to new file open");
345345
}
346346

347+
function recordCommands(fn) {
348+
const executed = [];
349+
const handler = function (event, id) { executed.push(id); };
350+
CommandManager.on(CommandManager.EVENT_BEFORE_EXECUTE_COMMAND, handler);
351+
try {
352+
fn();
353+
} finally {
354+
CommandManager.off(CommandManager.EVENT_BEFORE_EXECUTE_COMMAND, handler);
355+
}
356+
return executed;
357+
}
358+
359+
it("Should dispatch NAVIGATE_SHOW_IN_FILE_TREE when #showInfileTree is clicked", async function () {
360+
await openFile("test.css");
361+
const $btn = $("#showInfileTree");
362+
expect($btn.length).toBe(1);
363+
const executed = recordCommands(function () {
364+
$btn.trigger("click");
365+
});
366+
expect(executed).toContain(Commands.NAVIGATE_SHOW_IN_FILE_TREE);
367+
}, 15000);
368+
369+
it("Should dispatch CMD_FIND_IN_FILES when #searchNav is clicked", function () {
370+
const $btn = $("#searchNav");
371+
expect($btn.length).toBe(1);
372+
const executed = recordCommands(function () {
373+
$btn.trigger("click");
374+
});
375+
expect(executed).toContain(Commands.CMD_FIND_IN_FILES);
376+
}, 15000);
377+
347378
it("Should navigate back and forward between text files", async function () {
348379
await navigateResetStack();
349380
await _validateNavForFiles("test.css", "test.html", "test.js");

test/spec/SidebarTabs-integ-test.js

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -506,6 +506,52 @@ define(function (require, exports, module) {
506506

507507
expect(SidebarTabs.getActiveTab()).toBe(SidebarTabs.SIDEBAR_TAB_FILES);
508508
});
509+
510+
it("should scroll the selected tree node into view when re-invoked on an already-selected file", async function () {
511+
// FileTreeView only auto-scrolls on unselected→selected transitions,
512+
// so re-invoking NAVIGATE_SHOW_IN_FILE_TREE on an already-selected
513+
// file would otherwise be a no-op when the user has scrolled away.
514+
// handleShowInTree compensates by calling ViewUtils.scrollElementIntoView
515+
// after ProjectManager.showInTree resolves — this asserts that call fires.
516+
await awaitsForDone(
517+
SpecRunnerUtils.openProjectFiles(["test.js"]),
518+
"open test file"
519+
);
520+
// First invocation: sets up the selection. We don't spy here because
521+
// FileTreeView's own selection-transition scroll can also fire and we
522+
// want to isolate the explicit handleShowInTree scroll.
523+
await awaitsForDone(
524+
CommandManager.execute(Commands.NAVIGATE_SHOW_IN_FILE_TREE),
525+
"first show in file tree"
526+
);
527+
528+
const ViewUtils = testWindow.brackets.getModule("utils/ViewUtils");
529+
const originalScroll = ViewUtils.scrollElementIntoView;
530+
const calls = [];
531+
ViewUtils.scrollElementIntoView = function ($container, $element, scrollIntoView) {
532+
calls.push({
533+
containerId: $container && $container.attr && $container.attr("id"),
534+
hasSelectedNode: $element && $element.length > 0
535+
});
536+
return originalScroll.apply(this, arguments);
537+
};
538+
try {
539+
await awaitsForDone(
540+
CommandManager.execute(Commands.NAVIGATE_SHOW_IN_FILE_TREE),
541+
"re-invoke show in file tree"
542+
);
543+
// The selection-transition scroll in FileTreeView.componentDidUpdate
544+
// cannot fire here (file is already selected), so any scroll call
545+
// against #project-files-container must come from handleShowInTree.
546+
await awaitsFor(function () {
547+
return calls.some(function (c) {
548+
return c.containerId === "project-files-container" && c.hasSelectedNode;
549+
});
550+
}, "ViewUtils.scrollElementIntoView to be called with the selected tree node", 3000);
551+
} finally {
552+
ViewUtils.scrollElementIntoView = originalScroll;
553+
}
554+
});
509555
});
510556

511557
describe("SidebarView resize", function () {

0 commit comments

Comments
 (0)