[pull] latest from ag-grid:latest - #892
Merged
Merged
Conversation
* AG-18208 Resolve the columns tool panel's display name on read The Columns Tool Panel snapshotted each row's display name onto its ColumnModelItem when the tree was built, while every other consumer of that name resolved it live. Nothing owned or invalidated the snapshot, so any name change without a tree rebuild left it stale: rows recreated from the items — on a group expand/collapse, or on scroll recycling — rendered the pre-edit name again, and the panel's search box matched against it too. Drop the snapshot. ColumnModelItem.displayName resolves through ColumnNameService on read, and the two row components delegate to it instead of keeping their own copies, leaving one resolution path and nothing to invalidate. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * AG-18208 Tidy the tool-panel label test helper and name the search test for what it asserts The child-label helper is polled from waitFor, so its throwaway fallback row is now destroyed and detached after the label is read rather than accumulating duplicate labels in the grid DOM. The search test asserts that a rename made before the search text is set is matched, so its name says that. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * AG-18208 Cover tool-panel search over a virtualised, never-rendered column The search walks the whole tool-panel tree, not the rendered rows, so a column the virtual list never materialised is still matched — and matched on its live name. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * AG-18208 Memoise the tool-panel display name against its inputs Resolving on read kept a user headerValueGetter on the search path: every pass over the tree called it for every item, and each rendered row read the name several times. Cache it against the two things that can change it - colModel.colDefsVersion and the column/group header-name override - so it resolves once per change instead of once per read. * fix tests --------- Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Stephen Cooper <scooperdev@gmail.com>
* AG-18233 + AG-14491 * AG-18233 + AG-14491
Astro's ClientRouter keeps its history index and scroll offsets in
`history.state`. Its popstate handler returns early when that state is
null, so a bare `history.replaceState(null, '', url)` leaves back and
forward moving the address bar without ever swapping the document. A
partial state such as `{}` clears the null check but drops `index`,
which makes every traversal read as "back" and turns the router's next
push index into NaN.
Copying a heading's link icon or changing the demo toolbar's dropdown
both hit the `{}` case, corrupting history for the rest of the session.
Add `replaceHistoryUrl()` to ag-website-shared and route every call site
through it. It preserves the existing entry's state, and takes an
optional patch for pages that need to store their own state alongside
the router's bookkeeping. The file is identical to the copies in
ag-charts and ag-studio so the next subrepo sync is a no-op.
License-setup's search params switch from push to replace: the
checkbox filters the page rather than navigates it, and no page-level
popstate handler exists to service the entries a push creates.
The outgoing word was unmounted instantly with no exit animation, and the incoming one slid in from above over 0.25s on a curve that overshot its resting position, where the container's `overflow: hidden` clipped it. Swaps are now a handover: the outgoing word accelerates away and fades, and the incoming word decelerates into place behind it. Both travel a fraction of a line, because the line box is barely taller than the glyphs and a full-line carousel would slice through them while still opaque. The word present on first paint no longer animates, so nothing moves in the heading as the page loads.
…14889) The AG-17369 regression tests drive the filter through column-level api.setColumnFilterModel, but the reporter and QA (plnkr 41sqYmiFjrSjJN3S) use grid-level api.setFilterModel — which queues the model and replays it against the still-empty client-side row model while cellDataType inference is pending. Add a test exercising that entry point, reusing one columnDefs object across a destroy/recreate reset, asserting both dropdown-open orderings converge on the applied ['one'] model (one row, only 'one' ticked). Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* AG-15299: pin line-heights at call sites that misuse the type scale Only 19 of the 65 `line-height: var(--text-lh-*)` call sites in this repo pair a line-height token with its matching `--text-fs-*`. The rest apply a token to a different, inherited or fluid font size, so changing the tokens to absolute px — which is what AG-15299 asks for — would silently rewrite them. Give those sites the value they render today, so they no longer depend on the token's numeric value: - `--text-lh-*` used with a non-matching font size: replaced with the px value it computes to now (28px, 35px, 24px, 21px, 20px, 14px, 55px, 43px). - `.heroHeading` in BryntumCampaign keeps a ratio: its font-size is `clamp(48px, 5.5vw, 72px)` at the large breakpoint, so no fixed px line-height can track it. - `ContactResultPage .heroHeading` steps 36px -> 48px across a breakpoint, so its line-height steps with it. - Three sites needed a line-height added to a descendant. A unitless line-height is inherited as a ratio and re-multiplies per font-size; a px line-height is inherited as a fixed length. `campaigns .quote blockquote` and the docs intro `code` element previously re-multiplied and now say what they mean. Verified by measuring computed font-size and line-height for every text element across 20 pages, before and after: of 9,445 elements compared, 6 changed — the intended 14px 19.6px -> 20px rounding, max height delta 0.41px. Everything else is byte-identical. * AG-15299: split the inherited line-height ratio out of the type scale `--text-lh-base` is doing two incompatible jobs. `_typography.scss` pairs it with `--text-fs-base` on one element, but `_base.scss` also sets it on `html`, where it is the document-wide default that every element without its own line-height inherits. A unitless line-height re-multiplies against each descendant's own font-size; an absolute px one does not. That single line is what makes AG-15299 dangerous. Measured over 20 pages: turning the eight scale tokens into px changes 6,957 of 9,637 elements, because everything inheriting from `html` collapses onto one fixed length — 40px headings drop from a 48px to a 24px line box, 12px text jumps from 16.8px to 22px, and the largest element height shift is 167px. Introduce `--text-lh-ratio-base` for the consumers that need a ratio (the `html` default and the form elements, whose font-size comes from the user agent) and leave the `--text-lh-<step>` tokens for paired use. Re-measured with the same token flip on top: 419 of 9,637 elements change instead of 6,957, a 94% reduction, and the residue is 13 rule patterns where a scale token is used on a re-sized element. Inert on its own: `--text-lh-ratio-base` is 1.4, the same value `--text-lh-base` has today, so nothing renders differently until the tokens actually change. * AG-15299: reference the line-height token matching each font-size The previous commit pinned the mismatched call sites to the px value they rendered, which kept the pixels identical but dropped the design-system indirection at 13 sites. Point each one at the `--text-lh-*` belonging to its own `--text-fs-*` instead, so the scale stays a single source of truth. This is a deliberate visual change: those sites were referencing a token from a different step, so honouring the pairing moves them onto that step's ratio. Measured over 20 pages, 62 of 11,478 elements move: - 21 elements at 20px: 28px -> 24px line box (`--text-lh-base` -> `--text-lh-lg`), covering the campaign eyebrows, hero body, quote blockquote and the docs intro paragraph. - 10 elements at 32px: 35.2px -> 48px (`--text-lh-3xl` -> `--text-lh-2xl`) on the three campaign headings. The largest element grows 25.6px. - 5 elements at 16px: 24px -> 22.4px (`--text-lh-2xl` -> `--text-lh-base`) on the homepage and pricing quotes. The remainder is reflow around those changes plus known homepage animation jitter. Four sites keep a literal value because no token applies: - `BryntumCampaign .heroHeading` — `clamp(48px, 5.5vw, 72px)`, so the line box has to be a ratio. - `LicenseSetup .licencePlaceholder` (15px), `ContactResultPage .heroHeading` (36px/48px) and `license-pricing .price` (46px) declare raw font sizes that are not on the scale, so there is no matching token to pair with. A px line-height is what gives those an integral line box. The docs intro `code` was `font-size: 0.8em` of a 20px parent, i.e. exactly `--text-fs-base`; stating that makes the pairing explicit and survives the token change. * AG-15299: name the size-agnostic line-heights as ratios
* Load the example runner script in the vanilla JS template * Use the bundled example runner in exported vanilla JS examples * Call setUpPage in the vanilla JS example template
* AG-3390 Lint against raw history writes `no-restricted-properties` cannot express this: it resolves the object only for a bare identifier, so it matches `history.replaceState` but not `window.history.replaceState`. Use a `no-restricted-syntax` selector on the callee's property name instead, which catches both. The rule lives in the ag-website-shared config, so it is identical in every consumer of the subrepo and covers the shared components plus the helper's own directory (the helper itself is exempt). The docs config carries the same rule for site-local code. It immediately caught two live cases under public/scripts/, which the original sweep missed by only searching src/: - license-pricing.js replaced the state outright, so clicking the pricing CTA killed back/forward for the rest of the session. - beyond-the-prompt.js pushed a partial state, leaving `index` undefined, which makes the router read every later traversal as a "back" and turns its next push index into NaN. Both are classic scripts served from public/ with no module system, so they cannot import the helper and take inline suppressions instead. * AG-3390 Address review feedback on the history lint Scope the selector to browser history. Matching on the method name alone also rejected any unrelated object with a `pushState`/`replaceState` method - a state machine, a mock, a history-shaped abstraction. It now matches only a `history` receiver, in both the bare and `window`/`globalThis` qualified forms. Verified against all three real forms plus a same-named decoy. Lift the restriction list into the subrepo as eslint.history-rules.mjs and import it from both configs, so the rule has one definition per repo rather than a copy per config that can drift apart. Drop the `.ts`/`.tsx` files glob with it. The public/ scripts that motivated this rule are plain `.js`, so the glob excluded exactly the code most likely to get it wrong. Note `.astro` client scripts stay uncovered either way - there is no eslint-plugin-astro in the toolchain. Advance the index on the modal's pushed entry rather than copying it. Spreading the state alone left the new entry sharing the previous entry's index, and the router derives direction from `index`, so a forward traversal into the modal compared equal and still read as a "back". Mirror what the router writes on its own pushes.
* Update .gitrepo parent sha * git subrepo commit external/ag-website-shared subrepo: subdir: "external/ag-website-shared" merged: "33a9edfb59b" upstream: origin: "git@github.com:ag-grid/ag-website-shared.git" branch: "latest" commit: "c7e3b1a2514" git-subrepo: version: "0.4.9" origin: "https://github.com/ingydotnet/git-subrepo" commit: "5e0f401" * git subrepo push external/ag-website-shared subrepo/external/ag-website-shared subrepo: subdir: "external/ag-website-shared" merged: "33a9edfb59b" upstream: origin: "git@github.com:ag-grid/ag-website-shared.git" branch: "latest" commit: "33a9edfb59b" git-subrepo: version: "0.4.9" origin: "https://github.com/ingydotnet/git-subrepo" commit: "5e0f401"
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )