feat(time-field,time-picker,scroll-fade): new tedi-ready components #374 - #397

tedi/components/form/time-field/time-field.component.scss (1)

53-62: Remove !important declarations to resolve specificity issues.

The !important overrides on lines 54–57 indicate specificity conflicts with base button styles and can lead to maintenance issues. Consider:

1. Increasing selector specificity naturally (e.g., .tedi-time-field .tedi-time-field__icon)
2. Using CSS custom properties that the button component respects
3. Adjusting the base button component to be more customizable

The tedi-time-field__icon--static modifier class used in the HTML template (non-interactive state) does not have explicit styles, but this appears intentional—it's a semantic marker that correctly inherits from the base &__icon styles.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tedi/components/form/time-field/time-field.component.scss` around lines 53 -
62, Remove the four `!important` overrides on the `&__icon` rules to avoid
specificity fights; instead increase selector specificity (e.g., target
`.tedi-time-field .tedi-time-field__icon`) or rely on configurable CSS custom
properties the base button consumes (use the existing --button-xs-icon-size,
--form-field-button-height-sm, --button-radius-sm variables) so the icon styles
inherit normally; keep the `tedi-time-field__icon--static` modifier as-is since
it intentionally inherits from `&__icon`.

tedi/components/helpers/scroll-fade/scroll-fade.component.scss (1)

26-44: CSS nesting depth exceeds maximum of 4 levels.

The :hover pseudo-class inside &::-webkit-scrollbar-thumb creates 5 levels of nesting (.tedi-scroll-fade → &__inner → &--custom-scroll → &::-webkit-scrollbar-thumb → &:hover). As per coding guidelines, max CSS nesting depth is 4.

♻️ Proposed fix to flatten nesting

   &__inner {
     flex: 1;
     min-height: 0;
     max-height: inherit;
     overflow: auto;
     mask-image: linear-gradient(
       to bottom,
       transparent 0%,
       black var(--_tedi-scroll-fade-top),
       black calc(100% - var(--_tedi-scroll-fade-bottom)),
       transparent 100%
     );
-
-    &--custom-scroll {
-      &::-webkit-scrollbar {
-        width: 6px;
-        background-color: var(--general-surface-primary);
-      }
-
-      &::-webkit-scrollbar-thumb {
-        background: var(--general-border-primary);
-        border-radius: 100px;
-
-        &:hover {
-          background-color: var(--general-border-secondary);
-        }
-      }
-
-      &::-webkit-scrollbar-track {
-        background-color: var(--general-surface-primary);
-      }
-    }
   }
+
+  &__inner--custom-scroll {
+    &::-webkit-scrollbar {
+      width: 6px;
+      background-color: var(--general-surface-primary);
+    }
+
+    &::-webkit-scrollbar-thumb {
+      background: var(--general-border-primary);
+      border-radius: 100px;
+    }
+
+    &::-webkit-scrollbar-thumb:hover {
+      background-color: var(--general-border-secondary);
+    }
+
+    &::-webkit-scrollbar-track {
+      background-color: var(--general-surface-primary);
+    }
+  }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tedi/components/helpers/scroll-fade/scroll-fade.component.scss` around lines
26 - 44, The nesting depth exceeds the max because the &:hover pseudo inside
&::-webkit-scrollbar-thumb creates five levels; move the hover selector out to
flatten it. Concretely, remove the nested &:hover inside
&::-webkit-scrollbar-thumb and add a top-level selector targeting
&--custom-scroll::-webkit-scrollbar-thumb:hover (or the equivalent flattened
selector using &--custom-scroll and ::-webkit-scrollbar-thumb:hover) so you keep
the same styles but reduce nesting; update selectors referencing
&--custom-scroll, &::-webkit-scrollbar-thumb, and &:hover accordingly.

tedi/components/form/time-field/time-field.component.html (1)

29-49: Consider moving inline styles to CSS classes.

Lines 31 and 49 use inline style attributes. For consistency with the BEM approach and maintainability, consider using CSS classes instead.

♻️ Suggested approach

Add classes to the SCSS file:

.tedi-time-field__popover-trigger {
  display: flex;
  align-items: center;
}

.tedi-time-field__popover-content {
  padding: 0;
}

Then update the template:

     <tedi-popover
       `#popover`
-      style="display: flex; align-items: center"
+      class="tedi-time-field__popover-trigger"
       position="bottom-end"
...
-      <tedi-popover-content maxWidth="none" style="padding: 0">
+      <tedi-popover-content maxWidth="none" class="tedi-time-field__popover-content">

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tedi/components/form/time-field/time-field.component.html` around lines 29 -
49, Replace the inline styles on the tedi-popover host element and
tedi-popover-content with CSS classes: create classes (e.g.,
.tedi-time-field__popover-trigger and .tedi-time-field__popover-content) in the
component SCSS containing the styles "display: flex; align-items: center" and
"padding: 0" respectively, then remove the style attributes from the
<tedi-popover `#popover` ...> and <tedi-popover-content ...> elements and add the
corresponding class names to those elements to follow the BEM approach and keep
styling in SCSS.

tedi/components/form/time-field/time-field.component.ts (1)

194-203: Nested setTimeout pattern is fragile for timing coordination.

The double setTimeout relies on specific timing to wait for popover rendering and focus setup. This could be flaky if rendering takes longer than expected.

Consider using a more robust approach such as observing DOM changes or using Angular's lifecycle hooks if possible.

♻️ Consider using requestAnimationFrame or AfterViewInit pattern

   onPickerOpen() {
-    // First timeout waits for popover to render content in DOM.
-    // Nested timeout runs after popover's own focus setup (which also uses setTimeout).
-    setTimeout(() => {
-      this.timePicker()?.scrollToSelected();
-      setTimeout(() => {
-        this.timePicker()?.focusActiveItem();
-      });
-    });
+    // Use requestAnimationFrame to ensure DOM is ready
+    requestAnimationFrame(() => {
+      this.timePicker()?.scrollToSelected();
+      requestAnimationFrame(() => {
+        this.timePicker()?.focusActiveItem();
+      });
+    });
   }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tedi/components/form/time-field/time-field.component.ts` around lines 194 -
203, The nested setTimeouts in onPickerOpen (which calls
this.timePicker()?.scrollToSelected() and this.timePicker()?.focusActiveItem())
are fragile; replace them with a more robust coordination such as waiting for
Angular stabilization or the next animation frame: e.g., use NgZone.onStable or
requestAnimationFrame (or a MutationObserver if the popover content is dynamic)
to call scrollToSelected() after the popover DOM is rendered and then schedule
focusActiveItem() on the next animation frame (or when the zone is stable)
instead of using nested setTimeouts.

tedi/components/helpers/scroll-fade/scroll-fade.component.ts (1)

76-99: Consider using an arrow function for onScroll to ensure stable reference.

While onScroll is currently called via template binding (scroll)="onScroll()" which works correctly, the coding guidelines recommend arrow function properties for event handlers. This is a minor suggestion since the current implementation is functional.

♻️ Optional: Convert to arrow function

-  onScroll(): void {
+  onScroll = (): void => {
     const el = this.innerRef().nativeElement;
     this.updateFade(el.scrollTop, el.scrollHeight, el.clientHeight);
-  }
+  };

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tedi/components/helpers/scroll-fade/scroll-fade.component.ts` around lines 76
- 99, The onScroll method should be converted to an arrow-function property to
ensure a stable function reference for template bindings; replace the class
method onScroll() with an arrow property (keeping the same logic that reads
const el = this.innerRef().nativeElement and calls this.updateFade) so it
becomes onScroll = (): void => { ... }, leaving updateFade, ngAfterViewInit,
fade, scrolledToTop and scrolledToBottom unchanged; ensure any places that call
onScroll (template bindings) remain unchanged since the signature and behavior
are identical.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes