Skip to content

Commit 99cb0f9

Browse files
Copilotgortazar
andcommitted
fix: address add-channel picker review feedback
Agent-Logs-Url: https://github.com/gortazar/thunderbird-slack-provider/sessions/a2b942e7-1eb1-40d8-a067-47480e6b0b9f Co-authored-by: gortazar <203272+gortazar@users.noreply.github.com>
1 parent 971eb7a commit 99cb0f9

3 files changed

Lines changed: 15 additions & 4 deletions

File tree

src/space.html

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ <h2>Connect to Slack</h2>
6262
<div class="dialog">
6363
<h3 id="add-channel-title">Add Channel</h3>
6464
<p class="dialog-help">Pick a Slack channel to add.</p>
65-
<select id="add-channel-select" class="dialog-input"></select>
65+
<select id="add-channel-select" class="dialog-input" aria-label="Channel to add"></select>
6666
<div id="add-channel-error" class="error-msg hidden"></div>
6767
<div class="dialog-actions">
6868
<button id="btn-add-channel-cancel" class="btn btn-secondary">Cancel</button>

src/space.js

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ let disableAvatars = false;
1515
let rateLimitedMode = false;
1616
let workspaceName = "Slack";
1717
let addChannelChoices = [];
18+
let addChannelLoadId = 0;
1819

1920
// ---------------------------------------------------------------------------
2021
// Startup
@@ -377,18 +378,22 @@ function showAddChannelDialog() {
377378
const select = document.getElementById("add-channel-select");
378379
const errorEl = document.getElementById("add-channel-error");
379380
const confirmBtn = document.getElementById("btn-add-channel-confirm");
381+
const cancelBtn = document.getElementById("btn-add-channel-cancel");
382+
const loadId = ++addChannelLoadId;
380383
select.innerHTML = '<option value="">Loading channels...</option>';
381384
select.disabled = true;
382385
confirmBtn.disabled = true;
386+
confirmBtn.textContent = "Add Channel";
383387
errorEl.classList.add("hidden");
384388
dialog.classList.remove("hidden");
385-
select.focus();
389+
cancelBtn.focus();
386390
document.addEventListener("keydown", _onDialogKeydown);
387391

388392
Promise.all([
389393
bg({ type: "get_channels" }),
390394
bg({ type: "get_watched_channels" }),
391395
]).then(([chanRes, watchedRes]) => {
396+
if (loadId !== addChannelLoadId || dialog.classList.contains("hidden")) { return; }
392397
if (chanRes.error) {
393398
errorEl.textContent = `Could not load channels: ${chanRes.error}`;
394399
errorEl.classList.remove("hidden");
@@ -414,11 +419,13 @@ function showAddChannelDialog() {
414419
}
415420

416421
select.innerHTML = addChannelChoices
417-
.map((c) => `<option value="${escHtml(c.id)}">#${escHtml(c.name)}</option>`)
422+
.map((c) => `<option value="${escHtml(c.id)}">${c.is_private ? "🔒 " : "#"}${escHtml(c.name)}</option>`)
418423
.join("");
419424
select.disabled = false;
420425
confirmBtn.disabled = false;
426+
select.focus();
421427
}).catch((e) => {
428+
if (loadId !== addChannelLoadId || dialog.classList.contains("hidden")) { return; }
422429
errorEl.textContent = `Could not load channels: ${e.message}`;
423430
errorEl.classList.remove("hidden");
424431
select.innerHTML = '<option value="">No channels available</option>';
@@ -427,6 +434,8 @@ function showAddChannelDialog() {
427434

428435
function hideAddChannelDialog() {
429436
document.getElementById("add-channel-dialog").classList.add("hidden");
437+
document.getElementById("btn-add-channel-confirm").textContent = "Add Channel";
438+
addChannelLoadId++;
430439
addChannelChoices = [];
431440
document.removeEventListener("keydown", _onDialogKeydown);
432441
}

tests/e2e/options.spec.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -430,8 +430,10 @@ test.describe("space.html – v1.2.0 visual appearance", () => {
430430
await page.locator(".context-menu-item").filter({ hasText: "Add Channel" }).click();
431431
await expect(page.locator("#add-channel-dialog")).not.toHaveClass(/hidden/);
432432
await expect(page.locator("#add-channel-select")).toBeVisible();
433+
await expect(page.locator("#add-channel-select")).toHaveAttribute("aria-label", "Channel to add");
433434
await expect(page.locator("#add-channel-select")).toContainText("No channels available");
434435
await expect(page.locator("#btn-add-channel-confirm")).toBeDisabled();
436+
await expect(page.locator("#btn-add-channel-confirm")).toHaveText("Add Channel");
435437
await page.screenshot({ path: testInfo.outputPath("add-channel-dialog.png") });
436438
});
437439

@@ -455,7 +457,7 @@ test.describe("space.html – v1.2.0 visual appearance", () => {
455457
await page.locator(".workspace-header").hover();
456458
await page.locator(".workspace-menu-btn").click();
457459
await page.locator(".context-menu-item").filter({ hasText: "Add Channel" }).click();
458-
await expect(page.locator("#add-channel-select")).toContainText("#private");
460+
await expect(page.locator("#add-channel-select")).toContainText("🔒 private");
459461
await expect(page.locator("#add-channel-select")).toContainText("#random");
460462
await expect(page.locator("#add-channel-select")).not.toContainText("#general");
461463
await page.screenshot({ path: testInfo.outputPath("add-channel-dialog-with-options.png") });

0 commit comments

Comments
 (0)