Skip to content

fix: after update (#DS-5079)#1622

Draft
lskramarov wants to merge 12 commits into
mainfrom
fix/after-update
Draft

fix: after update (#DS-5079)#1622
lskramarov wants to merge 12 commits into
mainfrom
fix/after-update

Conversation

@lskramarov

Copy link
Copy Markdown
Contributor

No description provided.

@lskramarov lskramarov self-assigned this Jun 4, 2026
@lskramarov lskramarov requested a review from artembelik as a code owner June 4, 2026 10:42
@lskramarov lskramarov added the bug Something isn't working label Jun 4, 2026
@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

Visit the preview URL for this PR (updated for commit 6498b3f):

https://koobiq-next--prs-1622-815op42q.web.app

(expires Sat, 13 Jun 2026 16:38:07 GMT)

🔥 via Firebase Hosting GitHub Action 🌎

Sign: c9e37e518febda70d0317d07e8ceb35ac43c534c

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR hardens KbqSelect’s “hidden selected items” calculation logic to avoid runtime errors when expected trigger DOM elements aren’t present yet (regression observed after a framework update), and adds a unit test to prevent the crash from returning.

Changes:

  • Add null-guards in calculateHiddenItems when .kbq-select__match-hidden-text / .kbq-select__match-list aren’t available yet.
  • Make getTotalVisibleItems() tolerate a missing .kbq-select__match-hidden-text element before calling Renderer2.setStyle.
  • Add a regression unit test ensuring calculateHiddenItems() doesn’t throw when the hidden-text element is missing.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File Description
packages/components/select/select.component.ts Adds defensive null checks in hidden-items calculation paths to prevent DOM access / Renderer2.setStyle crashes.
packages/components/select/select.component.spec.ts Adds a regression test covering the “hidden-text not yet in DOM” scenario.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions

github-actions Bot commented Jun 4, 2026

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

@lskramarov lskramarov marked this pull request as draft June 5, 2026 06:31
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

Replace the legacy .eslintrc.js/.eslintignore with a flat eslint.config.js
and bump eslint to v9. Swap the unmaintained eslint-plugin-rxjs for the
maintained eslint-plugin-rxjs-x fork (rxjs-x/* namespace) and adopt the
typescript-eslint and angular-eslint umbrella packages.

Preserve the previous lint behavior: disable the rxjs-x rules new to its
recommended set (prefer-root-operators, prefer-observer), ignore .yarn
(flat config lints .cjs by default), and restore no-unused-vars
caughtErrors: 'none'.
Drop the align-self: flex-start override that lifted the toggle and checkbox off center.

Also center the badge in the tree-action-button example.
Run `ng generate @angular/core:inject` with --migrate-abstract-classes and
--backwards-compatible-constructors across all packages. Constructor parameter
injection is replaced by the inject() function.

Manual fixes for migration limitations:
- assert non-null (!) on all optional inject fields to preserve the original
  non-null @optional() typing and public API (nonNullableOptional behaviour)
- fix null/undefined mismatches (?? undefined) for luxon/moment options and
  markdown marked options
- form-field KbqTrim: coerce @Attribute('no-trim') to boolean at the field
- tooltip: fix KBQ_TOOLTIP_OPEN_TIME token type (() => ScrollStrategy -> { value: number })
- remove stale deps:[] from providers of inject()-based classes (formatters,
  notification-center, luxon/moment adapters) so Angular uses each class's
  factory and keeps the injection context (fixes NG0201)
- DateFormatter: inject the Core DateAdapter token instead of the base
  @koobiq/date-adapter one to match the registered provider
- adapt specs that used deps:[] / manual new KbqLocaleService() to the inject API

Builds, public API guard and unit tests (3121 passed) are green.
Remove the artifacts left by the --backwards-compatible-constructors run:
- the `/** Inserted by Angular inject() migration ... */` comments (152)
- the `constructor(...args: unknown[]);` overload signatures (152)
- empty `constructor() {}` and super-only `constructor() { super(); }` (78)

Subclasses that passed freshly-injected locals to an empty base constructor
(KbqFocusableComponent, KbqTreeBase, KbqTreeNode, KbqToastComponent) now call
super() with no arguments, since each base injects those dependencies itself;
the now-unused locals were removed. Public API constructor signatures change
from `(...args: unknown[])` to the real `()`.

Builds, public API guard and TypeScript compilation are green.
The backwards-compat scaffolding cleanup removed the constructor overload of
CdkVirtualScrollViewportSelect; its spec subclass passed scrollDispatcher to
super(). The base injects scrollDispatcher itself, so call super() with no args.
Extend the consumer-facing v20-upgrade codemod (auto-run via
`ng update @koobiq/components`, manual via `ng generate`) with the remaining
@deprecated symbols:

- DropdownPositionX/Y -> KbqDropdownPositionX/Y (TS type aliases)
- KbqCodeBlock [canLoad]/canLoad= -> canDownload, [codeFiles]/codeFiles= -> files
  (template attribute forms only; programmatic TS access surfaced as a warning)
- warn-only: scrollableCodeContent (use scrollTo()), KbqFilter.required
  (use cleanable=false + removable=false)

Adds spec coverage for each mapping and documents them in the migration README.
@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🚨 E2E tests failed

Review the report for details.


💡 Comment /approve-snapshots to approve snapshot changes.

@lskramarov

Copy link
Copy Markdown
Contributor Author

/approve-snapshots

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

🔄 Updating snapshots.

@github-actions

github-actions Bot commented Jun 8, 2026

Copy link
Copy Markdown

✅ Snapshots updated!

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

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants