Admin Panel / People: the tables are back, and the selected menu entry stands out.

Organizations, Teams and People drew their search box and their pager but NO TABLE.

Their wrappers said `+tablePage(orgTablePageData)` while that helper was registered on
Template.people - the PARENT template. Blaze resolves a name against the current
template's helpers, the global helpers and the data context; it never searches an
enclosing template. So the context came out undefined, and a table page with no
rowCount draws its chrome and stops. Nothing threw and every test passed.

Domains is the control that proves it: its helper sits on Template.domainGeneral, the
template that uses it, and Domains kept working the whole time.

The parent builds the context now - where the helper actually lives - and hands it
down, so each wrapper renders `+tablePage(this)`. The named arguments the wrappers
used to take (orgList, orgCurrentPage, hasOrgPrevPage, ...) went unused once these
panes moved to the shared page, so they go too.

A new test walks every `+tablePage(name)` in that file, works out which template it is
written in, and requires `name` to be a helper OF THAT TEMPLATE unless it is `this`.
Verified it flags `orgGeneral -> orgTablePageData` when the bug is put back.

Also: the selected left-menu entry is darker and bold, so the menu says at a glance
which page is showing on the right. Its colour fell back to `inherit`, which made it
exactly the same grey as every unselected entry - the white background and soft shadow
alone were easy to miss. A per-user theme accent still wins; only the fallback changed,
from `inherit` to a grey darker than the panel's. The icon takes the same colour as its
label.

Tests: 1347 passing.

Thanks to xet7 !

client/components/settings/peopleBody.jade

@@ -12,11 +12,11 @@ template(name="people")
           if loading.get
             +spinner
           else if orgSetting.get
-            +orgGeneral(orgList=orgList orgCurrentPage=orgCurrentPage orgTotalPages=orgTotalPages hasOrgPrevPage=hasOrgPrevPage hasOrgNextPage=hasOrgNextPage)
+            +orgGeneral(orgTablePageData)
           else if teamSetting.get
-            +teamGeneral(teamList=teamList teamCurrentPage=teamCurrentPage teamTotalPages=teamTotalPages hasTeamPrevPage=hasTeamPrevPage hasTeamNextPage=hasTeamNextPage)
+            +teamGeneral(teamTablePageData)
           else if peopleSetting.get
-            +peopleGeneral(peopleList=peopleList peopleCurrentPage=peopleCurrentPage peopleTotalPages=peopleTotalPages hasPeoplePrevPage=hasPeoplePrevPage hasPeopleNextPage=hasPeopleNextPage)
+            +peopleGeneral(peopleTablePageData)
           else if lockedUsersSetting.get
             +lockedUsersGeneral
           else if rolesSetting.get
@@ -28,7 +28,7 @@ template(name="people")
 
 
 template(name="orgGeneral")
-  +tablePage(orgTablePageData)
+  +tablePage(this)
 
 //- One header cell for an Organization feature column: the label plus its
 //- select-all / unselect-all pair. Rendered by the shared table page through the
@@ -44,7 +44,7 @@ template(name="orgFeatureHeader")
     i.fa.fa-square-o
 
 template(name="teamGeneral")
-  +tablePage(teamTablePageData)
+  +tablePage(this)
 
 //- The Teams equivalent of orgFeatureHeader: label plus its select-all pair,
 //- parameterised by feature. Rendered through the shared table page.
@@ -61,7 +61,7 @@ template(name="peopleGeneral")
   //- Not part of the table: the Add/Remove Teams panel this pane opens.
   #divAddOrRemoveTeamContainer
     +modifyTeamsUsers
-  +tablePage(peopleTablePageData)
+  +tablePage(this)
 
 template(name="selectAllUser")
   | {{_ 'dueCardsViewChange-choice-all'}}

client/components/settings/settingBody.css

@@ -45,12 +45,22 @@
   background: #fff;
   box-shadow: 0 2px 3px rgba(0,0,0,0.15);
 }
-/* The one themeable part of the menu: the entry you are on takes the per-user
-   theme accent (Member Settings -> Change color sets --theme-accent on :root).
-   The fallback is "inherit", so with no custom colour the menu looks exactly as
-   it always has - the WeKan default. See docs/Design/Page/Left-Menu.md. */
+/* The entry you are on - the one whose page is showing on the right - is DARKER and
+   BOLD, so the menu says where you are at a glance. The white background and shadow
+   above are subtle enough that, when the text was the same grey as every other entry,
+   the selected row was easy to miss.
+   Still the one themeable part of the menu: a per-user theme accent (Member Settings
+   -> Change color sets --theme-accent on :root) wins. The fallback is a darker grey
+   than the panel's #727479, not "inherit" - inheriting is what made it identical to
+   the unselected entries. See docs/Design/Page/Left-Menu.md. */
 .setting-content .content-body .side-menu ul li.active > a {
-  color: var(--theme-accent, inherit);
+  color: var(--theme-accent, #2c2e33);
+  font-weight: bold;
+}
+/* The icon takes the same colour: it is inside the <a>, so it inherits unless
+   something sets it, and stating it here keeps icon and label in step. */
+.setting-content .content-body .side-menu ul li.active > a i {
+  color: inherit;
 }
 .setting-content .content-body .side-menu ul li:hover {
   background: #fff;

docs/Design/Page/Left-Menu.md

@@ -74,16 +74,18 @@ Most of the menu is deliberately **neutral**: the panel is a light grey card
 soft shadow. That is the WeKan default look and it does not change with the theme
 — a menu is chrome, and tinting the whole card would fight the content beside it.
 
-The **themeable part is the active entry's label**:
+The **themeable part is the active entry's label**, which is also **darker and
+bold** so the menu says at a glance which page is showing on the right:
 
 ```css
-.side-menu ul li.active > a { color: var(--theme-accent, inherit); }
+.side-menu ul li.active > a { color: var(--theme-accent, #2c2e33); font-weight: bold; }
 ```
 
 - **Per-user theme** — Member Settings → Change color sets `--theme-accent` on
   `:root`, and the entry you are on picks it up, for that user only.
-- **WeKan default** — with no custom colour the fallback is `inherit`, so the
-  label keeps the normal text colour and the menu looks exactly as it always has.
+- **WeKan default** — the fallback is a grey darker than the panel's `#727479`.
+  It used to be `inherit`, which made the selected entry exactly the same colour as
+  every other one; the white background and soft shadow alone were easy to miss.
 
 So: nothing in the menu is hard-coded to a *brand* colour, and the one part that
 should follow a chosen theme does. If you add a themeable part, take the colour

tests/leftMenu.test.cjs

@@ -148,10 +148,19 @@ test('the design doc states which side, both ways', () => {
 
 // ── theme ───────────────────────────────────────────────────────────────────
 
-test('the active entry follows the theme, and the default look is unchanged', () => {
-  assert.ok(/\.side-menu ul li\.active > a \{\s*color: var\(--theme-accent, inherit\);/.test(css),
-    'the active entry takes the per-user accent, falling back to inherit so the ' +
-    'WeKan default look is untouched when no colour is chosen');
+test('the selected entry is darker and bold, and still themeable', () => {
+  // The white background and shadow on the active row are subtle; with the label the
+  // same grey as every other entry, which page you were on was easy to miss.
+  const active = /\.side-menu ul li\.active > a \{([^}]*)\}/.exec(css);
+  assert.ok(active, 'the active entry must be styled');
+  assert.ok(/font-weight:\s*bold/.test(active[1]), 'the selected label is bold');
+  assert.ok(/color: var\(--theme-accent, #[0-9a-f]{6}\)/.test(active[1]),
+    'a per-user accent still wins, over a DARKER fallback - "inherit" was what made ' +
+    'the selected entry identical to the unselected ones');
+  assert.ok(!/inherit/.test(active[1]), 'the fallback must not inherit the panel grey');
+  // The icon goes with the label rather than keeping its own colour.
+  assert.ok(/\.side-menu ul li\.active > a i \{[^}]*color: inherit/.test(css),
+    'the icon takes the same colour as the label');
   const menu = css.slice(css.indexOf('.side-menu {'), css.indexOf('.content-body .main-body {'));
   // No brand colour may be hard-coded into the menu.
   assert.ok(!/#0079bf|#2980b9|#01628c/.test(menu),

tests/tablePage.test.cjs

@@ -548,7 +548,7 @@ test('Organizations renders through the shared table page', () => {
   const people = read('client/components/settings/peopleBody.jade');
   const org = people.slice(people.indexOf('template(name="orgGeneral")'),
     people.indexOf('template(name="orgFeatureHeader")'));
-  assert.ok(/\+tablePage\(orgTablePageData\)/.test(org), 'renders the shared page');
+  assert.ok(/\+tablePage\(this\)/.test(org), 'renders the shared page from the context it was given');
   assert.ok(!/thead|org-pagination/.test(org), 'and keeps no table markup of its own');
   const js = read('client/components/settings/peopleBody.js');
   assert.ok(/rowTemplate: 'orgRow'/.test(js), 'its interactive rows use the row slot');
@@ -563,7 +563,7 @@ test('Teams renders through the shared table page, and gains a working prev', ()
   const people = read('client/components/settings/peopleBody.jade');
   const team = people.slice(people.indexOf('template(name="teamGeneral")'),
     people.indexOf('template(name="teamFeatureHeader")'));
-  assert.ok(/\+tablePage\(teamTablePageData\)/.test(team), 'renders the shared page');
+  assert.ok(/\+tablePage\(this\)/.test(team), 'renders the shared page from the context it was given');
   assert.ok(!/thead|team-pagination/.test(team), 'and keeps no table markup of its own');
   const js = read('client/components/settings/peopleBody.js');
   assert.ok(/rowTemplate: 'teamRow'/.test(js) && /headerTemplate: 'teamFeatureHeader'/.test(js));
@@ -579,7 +579,7 @@ test('the People pane renders through the shared table page', () => {
   const people = read('client/components/settings/peopleBody.jade');
   const pane = people.slice(people.indexOf('template(name="peopleGeneral")'),
     people.indexOf('template(name="selectAllUser")'));
-  assert.ok(/\+tablePage\(peopleTablePageData\)/.test(pane));
+  assert.ok(/\+tablePage\(this\)/.test(pane));
   assert.ok(!/thead|people-pagination/.test(pane), 'no table markup of its own');
   const js = read('client/components/settings/peopleBody.js');
   assert.ok(/rowTemplate: 'peopleRow'/.test(js));
@@ -659,4 +659,41 @@ test('People uses the shared controls row - search, filter, actions, total', ()
   }
 });
 
+test('every +tablePage(name) resolves where it is written', () => {
+  // Organizations, Teams and People rendered their search box and their pager but NO
+  // TABLE. Their wrappers said `+tablePage(orgTablePageData)` while that helper was
+  // registered on Template.people - the PARENT. Blaze resolves a name against the
+  // current template's helpers, the global helpers and the data context; it never
+  // searches an enclosing template. So the context was undefined, and a table page
+  // with no rowCount draws its chrome and stops. Nothing threw, and every test passed.
+  //
+  // Domains is the control: its helper sits on Template.domainGeneral, the template
+  // that uses it, and Domains kept working throughout.
+  const jadeSrc = read('client/components/settings/peopleBody.jade');
+  const jsSrc = read('client/components/settings/peopleBody.js');
+  // Which template each `+tablePage(...)` call is inside.
+  const templates = [];
+  for (const line of jadeSrc.split('\n')) {
+    const t = /^template\(name="(\w+)"\)/.exec(line);
+    if (t) templates.push({ name: t[1], args: [] });
+    const call = /\+tablePage\((\w+)\)/.exec(line);
+    if (call && templates.length) templates[templates.length - 1].args.push(call[1]);
+  }
+  const calls = templates.filter(t => t.args.length);
+  assert.ok(calls.length >= 4, 'the People panes must render the shared table page');
+  for (const { name, args } of calls) {
+    for (const arg of args) {
+      // `this` is the context handed down by the parent - always resolvable.
+      if (arg === 'this') continue;
+      // Otherwise the helper must be registered on THIS template.
+      const owner = new RegExp(`Template\\.${name}\\.helpers\\(\\{[\\s\\S]*?\\n\\}\\);`)
+        .exec(jsSrc);
+      assert.ok(owner && owner[0].includes(`${arg}()`),
+        `${name} renders +tablePage(${arg}) but ${arg} is not a helper of ${name} - `
+        + 'Blaze will not find it on an enclosing template, and the table silently '
+        + 'disappears');
+    }
+  }
+});
+
 console.log(`\ntablePage: ${passed} tests passed`);