Skip to content

docs(workbooks): document table chart vertical alignment for cell values - #11447

Open
igorlukanin wants to merge 1 commit into
masterfrom
igor/cub-3616-table-vertical-alignment-docs
Open

docs(workbooks): document table chart vertical alignment for cell values#11447
igorlukanin wants to merge 1 commit into
masterfrom
igor/cub-3616-table-vertical-alignment-docs

Conversation

@igorlukanin

@igorlukanin igorlukanin commented Aug 1, 2026

Copy link
Copy Markdown
Member

Summary

  • The table chart is gaining a vertical alignment control for cell values (top / middle / bottom), alongside the horizontal one it already has. The control is always available; its effect only shows once a row is taller than one line — with word wrap on, or with tall cell content.
  • Documents it in both places the table page covers alignment: the per-column options table and the Headers/Values/Totals formatting toolbar list, noting that it applies to Values only (headers and totals are always a single line).

Test plan

  • Wording matches the shipped control labels (Top / Middle / Bottom) and what an unset control renders (top)
  • CI / docs preview renders

@igorlukanin
igorlukanin requested a review from keydunov as a code owner August 1, 2026 13:12
@mintlify

mintlify Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated (UTC)
cubed3 🟢 Ready View Preview Aug 1, 2026, 1:13 PM

💡 Tip: Enable Workflows to automatically generate PRs for you.

@claude

claude Bot commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 59s —— View job


Review complete — docs-only change, looks good. 0 high, 0 medium, 2 low (both inline).

Details
  • Read PR diff (git diff origin/master...HEAD)
  • Read the full table.mdx page for context and consistency
  • Checked existing review threads (none)
  • Posted review

Assessment

Docs-only, 4 added lines to docs-mintlify/docs/explore-analyze/charts/chart-types/table.mdx. No code, so the security / performance / test-coverage axes don't apply. The change lands in the right file (active Mintlify site, not legacy /docs), needs no docs.json update since it's an existing page, and follows the repo's style conventions: table row added next to the related Alignment option, bold UI labels, second person, no new headings or padding.

Content-wise it's accurate and well-scoped — it documents the feature in both places the page already covers alignment (per-column table at line 37, formatting toolbar at line 208), states the default (middle), and notes that per-column overrides table-wide.

Findings (both low, both inline)

# Location Issue
1 table.mdx:213 "only becomes visible" is ambiguous — the PR description says the control is hidden until rows are multi-line, but the sentence reads as being about the effect. Also, the paragraph points at Word wrap (a per-column option) while sitting in the section whose equivalent control is Overflow — sends the reader to a different panel than the section describes.
2 table.mdx:208 Small redundancy/placement: the single-line rationale appears in both the bullet and the paragraph at 213, and the unrelated color sentence at 211 now sits between the bullet and its explanation. Suggested a tightened bullet.

Neither blocks merge — the second is purely editorial, and the first is a one-sentence clarification worth making since it's the thing a reader hunting for the control will trip over.

• [Branch `igor/cub-3616-table-vertical-alignment-docs`](https://github.com/cube-js/cube/tree/igor/cub-3616-table-vertical-alignment-docs)

Comment thread docs-mintlify/docs/explore-analyze/charts/chart-types/table.mdx Outdated
Comment thread docs-mintlify/docs/explore-analyze/charts/chart-types/table.mdx Outdated
@claude

claude Bot commented Aug 2, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 50s —— View job


Re-review after 9d66474 — docs-only, still good. 0 high, 0 medium, 2 low (both already open as inline threads, re-affirmed).

Details
  • Read the new commit (git show 9d66474) and the full PR diff
  • Re-read table.mdx around lines 34–42 and 201–221 for context
  • Listed existing review threads and deduplicated
  • Posted updated summary

What changed since the last review

One line, in docs-mintlify/docs/explore-analyze/charts/chart-types/table.mdx:213:

-... The default is middle, and a per-column setting overrides the table-wide one.
+... Until you pick one, values sit at the top. A per-column setting overrides the table-wide one.

Accurate-looking correction, and phrasing it as "until you pick one, values sit at the top" reads better than a bare "the default is …". No other lines touched.

Findings

Both prior low-severity threads still apply verbatim — the paragraph text they cover is unchanged apart from the default sentence, so I did not repost them:

# Location Status Issue
1 table.mdx:213 open (marked outdated by GitHub only because line numbers shifted) "only becomes visible" is ambiguous (control vs. effect), and the paragraph points at Word wrap (per-column, line 38) while sitting in the section whose control is Overflow (line 209).
2 table.mdx:208 open The single-line rationale lands twice (bullet + line 213), and the color sentence at 211 sits between the bullet and its explanation.

One new, non-blocking note: the PR description's test plan still says the wording matches "the default (middle)" — that box is now stale given this commit changes it to top. Worth updating the description so a future reader of the PR history doesn't take the checked box as evidence for the old value.

Nothing here blocks merge: both findings are editorial, and the correctness question (which value is the actual default) has been answered by this commit.

Branch igor/cub-3616-table-vertical-alignment-docs

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 1m 10s —— View job


Re-review after bbc40fa — both prior findings addressed. 0 high, 0 medium, 1 low (inline).

Details
  • Read the current state of table.mdx (full page, for surrounding context)
  • Listed existing review threads and deduplicated
  • Verified both prior low-severity findings are resolved in the current text
  • Posted review

What changed

bbc40fa responds to both of my earlier threads in docs-mintlify/docs/explore-analyze/charts/chart-types/table.mdx:

Thread 1 (control vs. effect, Word wrap vs Overflow) — resolved. The clarification that the control always renders and only its effect is conditional is now explicit, and the sentence names the section-local Overflow control with the per-column Word wrap as a cross-reference, so it no longer points readers at a different panel than the section describes (line 211):

Vertical alignment is always available, but it only has a visible effect once a row is taller than one line — set Overflow to wrap, or turn on Word wrap for an individual column, …

Worth noting the correction in the reply: the control is not conditionally hidden. My original comment hedged between the two readings based on the PR description ("only becomes visible once a row is taller than one line"), and the text now says which is true — that's the more useful version for a reader hunting for the control.

Thread 2 (redundancy / placement) — resolved. The bullet at line 208 is the tightened one-line form, and the explanatory paragraph moved to line 211, directly under the bullet list, so the color sentence (now line 213) no longer splits the bullet from its explanation.

Both threads are already marked resolved, so nothing to clean up.

Findings

# Location Severity Issue
1 table.mdx:211 low "Until you pick one" has a loose antecedent — the nearest candidates in the preceding clause are Overflow and Word wrap, not the alignment value, so it can read as "until you turn on wrapping." Suggested "Until you set a vertical alignment, …" inline.

Consistency of the rest checks out: the per-column table row at line 37 and the toolbar bullet at line 208 use the same three values in the same order and the same bolded-label style, Values-only scoping is stated once, and the default (top) is stated once rather than in both places.

One stale-metadata note carried over from the last review: the PR description's test plan still says the wording matches "the default (middle)", which 9d66474 changed to top. Worth editing the description so the checked box isn't read later as evidence for the old value.

Nothing blocks merge. Docs-only change in the active Mintlify site, no docs.json update needed (existing page), so the security / performance / test-coverage axes don't apply.

• [Branch `igor/cub-3616-table-vertical-alignment-docs`](https://github.com/cube-js/cube/tree/igor/cub-3616-table-vertical-alignment-docs)

Comment thread docs-mintlify/docs/explore-analyze/charts/chart-types/table.mdx Outdated
@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 50s —— View job


Re-review after 6570638 — all prior findings resolved. 0 high, 0 medium, 0 low. LGTM.

Details
  • Read the current state of table.mdx (per-column table + Style options section)
  • Listed existing review threads (gh list-review-threads) and confirmed all three of mine are resolved
  • Verified the last commit's wording against the earlier suggestion
  • Posted review

What changed

6570638 takes the suggestion from my previous review verbatim, in docs-mintlify/docs/explore-analyze/charts/chart-types/table.mdx:211:

-... Until you pick one, values sit at the top. A per-column setting overrides the table-wide one.
+... Until you set a vertical alignment, values sit at the top. A per-column setting overrides the table-wide one.

That removes the loose antecedent — the sentence now names the setting instead of leaning on "one", so it can no longer be misread as "until you turn on wrapping."

Findings

None. All three prior threads (control-vs-effect ambiguity, Word wrap vs Overflow naming, and the bullet redundancy/placement) are resolved in the current text, and I found nothing new.

Final state of the change, 3 content lines:

Line Content
37 Per-column Vertical alignment row — "Top, middle, or bottom — where the value sits when the row is taller than one line"
208 Toolbar bullet — "top, middle, or bottom (Values only; headers and totals are always a single line)"
211 Explanatory paragraph — always-available control, conditional effect, Overflow / Word wrap cross-reference, default (top), per-column override

Consistency checks pass: same three values in the same order in both places, same bolded-UI-label style as the surrounding page, Values-only scoping and the default each stated exactly once, and the explanation sits directly under the bullet list rather than splitting a bullet from its rationale. Docs-only change in the active Mintlify site (/docs-mintlify, not legacy /docs), existing page so no docs.json update needed — the security, performance, and test-coverage axes don't apply.

One piece of stale metadata (non-blocking, outside the diff)

The PR description's test plan still reads "the default (middle)", which 9d66474 corrected to top. Worth editing the description so the checked box isn't later read as evidence for the old value.

Branch igor/cub-3616-table-vertical-alignment-docs

@claude

claude Bot commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 35s —— View job


Re-review after force-push to 35fa0ec — docs-only, all prior findings resolved. 0 high, 0 medium, 0 low. LGTM.

Details
  • Read the PR diff (git diff origin/master...HEAD) and commit list
  • Read table.mdx around lines 35–48 and 207–239 for surrounding context
  • Listed review threads (gh list-review-threads) — all three of mine are resolved, none to clean up
  • Posted review

What changed

The branch was squashed: the four commits from the earlier rounds (9d66474, bbc40fa, 6570638, …) are now a single commit 35fa0ec with the same final content. The diff is still 4 added lines, one file, no deletions — nothing new was introduced by the squash.

Current state, with the line numbers as they now sit:

Line Content
43 Per-column Vertical alignment row — "Top, middle, or bottom — where the value sits when the row is taller than one line"
218 Toolbar bullet — "top, middle, or bottom (Values only; headers and totals are always a single line)"
231 Explanatory paragraph — always-available control, conditional effect, Overflow / Word wrap cross-reference, default (top), per-column override

Verification

All three earlier threads stay fixed through the squash:

  1. Control-vs-effect ambiguity — line 231 says the control "is always available, but it only has a visible effect once a row is taller than one line". Explicit.
  2. Word wrap vs Overflow — the same sentence names the section-local Overflow first and the per-column Word wrap as the cross-reference, so it no longer points readers at a different panel than the section describes.
  3. Loose antecedent — "Until you set a vertical alignment, values sit at the top" names the setting rather than leaning on "one".

Consistency checks pass: same three values in the same order in both places, same bolded-UI-label style as the surrounding page, Values-only scoping and the default each stated exactly once. The explanatory paragraph at 231 sits after the <Note> about Values alignment (223–229) rather than immediately under the bullet list — that reads fine, since the horizontal-alignment discussion and its note finish before the vertical one begins, and the color sentence no longer splits a bullet from its rationale.

Docs-only change in the active Mintlify site (/docs-mintlify, not legacy /docs), existing page so no docs.json update is needed — the security, performance, and test-coverage axes don't apply.

One piece of stale metadata (non-blocking, outside the diff)

Carried over from earlier rounds: the PR description's test plan still reads "the default (middle)", which the final text corrects to top. Worth editing the description so the checked box isn't later read as evidence for the old value.

Branch igor/cub-3616-table-vertical-alignment-docs

@igorlukanin
igorlukanin force-pushed the igor/cub-3616-table-vertical-alignment-docs branch from 35fa0ec to 9f76c0c Compare August 4, 2026 16:01
@claude

claude Bot commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Claude finished @igorlukanin's task in 36s —— View job


Re-review after rebase onto c7a1275 — content unchanged, all prior findings resolved. 0 high, 0 medium, 0 low. LGTM.

Details
  • Read the PR diff (git diff origin/master...HEAD) and commit list
  • Read table.mdx around lines 35–48 and 220–260 for surrounding context
  • Listed review threads (gh list-review-threads) — all three of mine are resolved, none to clean up or repost
  • Posted review

What changed

Nothing in the content. This synchronize is a rebase: the branch is now a single commit 9f76c0c sitting on top of the new master tip c7a1275 ("docs(table): document conditional-formatting indicator icons"). The diff against origin/master is still 4 added lines, one file, no deletions.

Because master picked up the conditional-formatting docs, the line numbers moved again:

Line Content
43 Per-column Vertical alignment row — "Top, middle, or bottom — where the value sits when the row is taller than one line"
232 Toolbar bullet — "top, middle, or bottom (Values only; headers and totals are always a single line)"
245 Explanatory paragraph — always-available control, conditional effect, Overflow / Word wrap cross-reference, default (top), per-column override

Verification

All three earlier findings stay fixed through the rebase:

  1. Control-vs-effect ambiguity — line 245 says the control "is always available, but it only has a visible effect once a row is taller than one line". Explicit.
  2. Word wrap vs Overflow — the same sentence names the section-local Overflow first, with the per-column Word wrap as the cross-reference, so it no longer sends readers to a different panel than the section describes.
  3. Loose antecedent — "Until you set a vertical alignment, values sit at the top" names the setting rather than leaning on "one".

Consistency checks pass: the same three values in the same order in both places, the same bolded-UI-label style as the surrounding page, and Values-only scoping plus the default each stated exactly once. The new upstream conditional-formatting content doesn't collide with this change — it lives in its own section and doesn't discuss alignment, so nothing needed reconciling.

Docs-only change in the active Mintlify site (/docs-mintlify, not legacy /docs), existing page so no docs.json update is needed — the security, performance, and test-coverage axes don't apply.

One piece of stale metadata (non-blocking, outside the diff)

Carried over from earlier rounds: the PR description's test plan still reads "the default (middle)", which the shipped text corrects to top. Worth editing the description so the checked box isn't later read as evidence for the old value.

Branch igor/cub-3616-table-vertical-alignment-docs

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant