feat(horizontal-stepper): new TEDI-ready component #405#410
Closed
mart-sessman wants to merge 0 commit into
Closed
feat(horizontal-stepper): new TEDI-ready component #405#410mart-sessman wants to merge 0 commit into
mart-sessman wants to merge 0 commit into
Conversation
mart-sessman
temporarily deployed
to
github-pages
GitHub Actions
Inactive
Contributor
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughA new horizontal stepper navigation component is introduced, consisting of a parent Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
tedi/components/navigation/horizontal-stepper/horizontal-stepper.component.ts (1)
13-26: Missingstandalone: truein component decorator.Per coding guidelines, all components must explicitly use
standalone: true. While Angular 19+ defaults to standalone, the project conventions require explicit declaration for consistency.♻️ Proposed fix
`@Component`({ selector: "tedi-horizontal-stepper", + standalone: true, templateUrl: "./horizontal-stepper.component.html", styleUrl: "./horizontal-stepper.component.scss",As per coding guidelines: "Use
standalone: truefor all components".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tedi/components/navigation/horizontal-stepper/horizontal-stepper.component.ts` around lines 13 - 26, The `@Component` decorator for the horizontal stepper is missing the required standalone: true flag; update the component decorator in horizontal-stepper.component.ts (the `@Component` that decorates the HorizontalStepperComponent) to include standalone: true alongside the existing metadata so it explicitly declares the component as standalone per project conventions.tedi/components/navigation/horizontal-stepper/horizontal-stepper.stories.ts (2)
155-167: Consider bindingbackgroundto args for consistency.The
TransparentBackgroundstory hardcodesbackground="transparent"in the template. While this works, it means the Storybook control won't affect this story, unlike other stories that use[background]="background".♻️ Optional fix for consistency
export const TransparentBackground: Story = { render: (props) => ({ props, template: ` - <tedi-horizontal-stepper ariaLabel="Form progress" background="transparent"> + <tedi-horizontal-stepper ariaLabel="Form progress" [background]="background"> <tedi-horizontal-stepper-item label="Kutse" completed /> <tedi-horizontal-stepper-item label="Tahteavaldus" selected /> <tedi-horizontal-stepper-item label="Geenianalüüs" /> <tedi-horizontal-stepper-item label="Vastus" /> </tedi-horizontal-stepper> `, }), + args: { + background: "transparent", + }, };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tedi/components/navigation/horizontal-stepper/horizontal-stepper.stories.ts` around lines 155 - 167, The TransparentBackground story hardcodes background="transparent" which prevents Storybook controls from altering it; update the story render/template for TransparentBackground to bind the input to the story arg (use [background]="background") and ensure the Story object exposes a background arg (e.g., default value 'transparent') so the control can modify it at runtime; locate the TransparentBackground Story definition and change its template binding from background="transparent" to [background]="background" and add the background arg/default to the Story config.
10-38: Add status parameters to story metadata.Per coding guidelines, Storybook stories should include status parameters to indicate the component's readiness state.
♻️ Proposed fix
export default { title: "TEDI-Ready/Components/Navigation/HorizontalStepper", component: HorizontalStepperComponent, + parameters: { + status: { + type: "devComponent", // or 'partiallyTediReady', 'existsInTediReady' as appropriate + }, + }, decorators: [Based on learnings: "Use status parameters in Storybook stories (e.g.,
partiallyTediReady,existsInTediReady,devComponent)".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tedi/components/navigation/horizontal-stepper/horizontal-stepper.stories.ts` around lines 10 - 38, Update the default export metadata for HorizontalStepperComponent to include Storybook status parameters indicating readiness: add a parameters block (inside the export default) with a status value such as { type: "partiallyTediReady" } (or use "existsInTediReady"/"devComponent" as appropriate for this component) so the exported object for HorizontalStepperComponent contains parameters.status alongside title/component/decorators/argTypes; ensure you modify the export default object in horizontal-stepper.stories.ts (the object referencing HorizontalStepperComponent and HorizontalStepperItemComponent).tedi/components/navigation/horizontal-stepper/horizontal-stepper-item/horizontal-stepper-item.component.ts (1)
13-27: Missingstandalone: truein component decorator.Same issue as the parent component - the
standalone: trueproperty should be explicitly declared.♻️ Proposed fix
`@Component`({ selector: "tedi-horizontal-stepper-item", + standalone: true, imports: [IconComponent, TediTranslationPipe], templateUrl: "./horizontal-stepper-item.component.html",As per coding guidelines: "Use
standalone: truefor all components".🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tedi/components/navigation/horizontal-stepper/horizontal-stepper-item/horizontal-stepper-item.component.ts` around lines 13 - 27, The `@Component` decorator for the "tedi-horizontal-stepper-item" component is missing standalone: true; open horizontal-stepper-item.component.ts, locate the `@Component` block (selector: "tedi-horizontal-stepper-item") and add standalone: true alongside the existing imports array so the component becomes standalone (preserve existing imports: [IconComponent, TediTranslationPipe], templateUrl, styleUrl, encapsulation, changeDetection, and host entries); ensure the change is consistent with the parent horizontal-stepper component.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In
`@tedi/components/navigation/horizontal-stepper/horizontal-stepper.component.scss`:
- Around line 5-6: The modifier "&--transparent" in
horizontal-stepper.component.scss currently uses a hardcoded "transparent"
value; replace that hardcoded color with the appropriate design token/CSS
variable from the design system (import or reference the token from
`@tedi-design-system/core`) and apply the token to the background-color for the
&--transparent modifier so the component uses the centralized color token
instead of a literal value.
---
Nitpick comments:
In
`@tedi/components/navigation/horizontal-stepper/horizontal-stepper-item/horizontal-stepper-item.component.ts`:
- Around line 13-27: The `@Component` decorator for the
"tedi-horizontal-stepper-item" component is missing standalone: true; open
horizontal-stepper-item.component.ts, locate the `@Component` block (selector:
"tedi-horizontal-stepper-item") and add standalone: true alongside the existing
imports array so the component becomes standalone (preserve existing imports:
[IconComponent, TediTranslationPipe], templateUrl, styleUrl, encapsulation,
changeDetection, and host entries); ensure the change is consistent with the
parent horizontal-stepper component.
In
`@tedi/components/navigation/horizontal-stepper/horizontal-stepper.component.ts`:
- Around line 13-26: The `@Component` decorator for the horizontal stepper is
missing the required standalone: true flag; update the component decorator in
horizontal-stepper.component.ts (the `@Component` that decorates the
HorizontalStepperComponent) to include standalone: true alongside the existing
metadata so it explicitly declares the component as standalone per project
conventions.
In `@tedi/components/navigation/horizontal-stepper/horizontal-stepper.stories.ts`:
- Around line 155-167: The TransparentBackground story hardcodes
background="transparent" which prevents Storybook controls from altering it;
update the story render/template for TransparentBackground to bind the input to
the story arg (use [background]="background") and ensure the Story object
exposes a background arg (e.g., default value 'transparent') so the control can
modify it at runtime; locate the TransparentBackground Story definition and
change its template binding from background="transparent" to
[background]="background" and add the background arg/default to the Story
config.
- Around line 10-38: Update the default export metadata for
HorizontalStepperComponent to include Storybook status parameters indicating
readiness: add a parameters block (inside the export default) with a status
value such as { type: "partiallyTediReady" } (or use
"existsInTediReady"/"devComponent" as appropriate for this component) so the
exported object for HorizontalStepperComponent contains parameters.status
alongside title/component/decorators/argTypes; ensure you modify the export
default object in horizontal-stepper.stories.ts (the object referencing
HorizontalStepperComponent and HorizontalStepperItemComponent).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 905dfafe-3f17-4f8a-9025-05693c5fdae0
📒 Files selected for processing (13)
tedi/components/navigation/horizontal-stepper/horizontal-stepper-item/horizontal-stepper-item.component.htmltedi/components/navigation/horizontal-stepper/horizontal-stepper-item/horizontal-stepper-item.component.scsstedi/components/navigation/horizontal-stepper/horizontal-stepper-item/horizontal-stepper-item.component.spec.tstedi/components/navigation/horizontal-stepper/horizontal-stepper-item/horizontal-stepper-item.component.tstedi/components/navigation/horizontal-stepper/horizontal-stepper-item/index.tstedi/components/navigation/horizontal-stepper/horizontal-stepper.component.htmltedi/components/navigation/horizontal-stepper/horizontal-stepper.component.scsstedi/components/navigation/horizontal-stepper/horizontal-stepper.component.spec.tstedi/components/navigation/horizontal-stepper/horizontal-stepper.component.tstedi/components/navigation/horizontal-stepper/horizontal-stepper.stories.tstedi/components/navigation/horizontal-stepper/index.tstedi/components/navigation/index.tstedi/services/translation/translations.ts
Comment on lines
+5
to
+6
| &--transparent { | ||
| background-color: transparent; |
There was a problem hiding this comment.
Replace hardcoded transparent color with a design token.
Line 6 uses transparent directly. Please map this modifier background to a design token/CSS variable from the design system instead of a hardcoded color value.
🎯 Suggested update
.tedi-horizontal-stepper {
display: flex;
background-color: var(--stepper-background);
&--transparent {
- background-color: transparent;
+ background-color: var(--stepper-background-transparent);
}
}As per coding guidelines, “Do not hardcode color values — always use design tokens from @tedi-design-system/core.”
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In
`@tedi/components/navigation/horizontal-stepper/horizontal-stepper.component.scss`
around lines 5 - 6, The modifier "&--transparent" in
horizontal-stepper.component.scss currently uses a hardcoded "transparent"
value; replace that hardcoded color with the appropriate design token/CSS
variable from the design system (import or reference the token from
`@tedi-design-system/core`) and apply the token to the background-color for the
&--transparent modifier so the component uses the centralized color token
instead of a literal value.
d97d995 to
5a9d803
Compare
2 tasks
3 tasks
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Summary by CodeRabbit