-
Notifications
You must be signed in to change notification settings - Fork 1
feat(pds-textarea): add highlight prop and styling #569
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
✅ Deploy Preview for pine-design-system ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis pull request updates the core component type definitions to add @default documentation annotations and mark several LocalJSX props as optional across multiple components. Concurrently, it introduces a new optional Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (6)
libs/core/src/components/pds-textarea/readme.md (1)
19-19: Clarify state precedence in prop docs.Note that highlight is ignored when disabled, readonly, or invalid. Add this to the JSDoc so it appears in the auto-generated table.
libs/core/src/components/pds-textarea/test/pds-textarea.e2e.ts (1)
227-261: Good coverage for reflection/toggling. Consider adding precedence cases.Add E2E cases ensuring highlight is ignored when readonly and when invalid.
Example to append:
+ it('ignores highlight when readonly', async () => { + const page = await newE2EPage(); + await page.setContent('<pds-textarea highlight readonly label="Msg" value="x"></pds-textarea>'); + const host = await page.find('pds-textarea'); + const textarea = await page.find('pds-textarea >>> textarea'); + expect(host).toHaveAttribute('highlight'); + expect(await textarea.getProperty('readOnly')).toBe(true); + }); + + it('ignores highlight when invalid', async () => { + const page = await newE2EPage(); + await page.setContent('<pds-textarea highlight invalid error-message="err" value="x"></pds-textarea>'); + const host = await page.find('pds-textarea'); + const field = await page.find('pds-textarea >>> .pds-textarea__field'); + expect(host).toHaveAttribute('highlight'); + expect(await field.getProperty('className')).toContain('is-invalid'); + });libs/core/src/components/pds-textarea/docs/pds-textarea.mdx (1)
119-129: Use boolean prop in React example and call out precedence.
- In React, prefer boolean prop: highlight or highlight={true} instead of highlight="true".
- Add a short note: “Highlight is ignored when disabled, readonly, or invalid.”
Suggested snippet change:
- react: '<PdsTextarea name="Highlight" label="Message" highlight="true" value="This textarea is highlighted" componentId="textarea-highlight"></PdsTextarea>', + react: '<PdsTextarea name="Highlight" label="Message" highlight value="This textarea is highlighted" componentId="textarea-highlight"></PdsTextarea>',libs/core/src/components/pds-textarea/pds-textarea.scss (1)
13-28: Scope counter highlight to non-invalid state to avoid mixed styles.When invalid, the field drops highlight but the counter still picks highlight text color. Scope the counter rule to non-invalid too.
- .pds-textarea__character-counter { - background: color-mix(in srgb, var(--pine-color-accent-disabled) 80%, transparent); - color: var(--pine-color-purple-700); - } + // Only apply counter highlight when field is not invalid + .pds-textarea__field:not(.is-invalid) ~ .pds-textarea__character-counter { + background: color-mix(in srgb, var(--pine-color-accent-disabled) 80%, transparent); + color: var(--pine-color-purple-700); + }Optional:
- Prefer an accent text token (e.g., --pine-color-text-accent) over a specific purple scale to support theming.
libs/core/src/components/pds-textarea/pds-textarea.tsx (1)
153-157: Prop looks good. Add JSDoc details for precedence and default.Document that highlight is ignored when disabled, readonly, or invalid, and add @DefaultValue for consistency with other props.
/** - * Applies highlight styling to the textarea field. + * Applies highlight styling to the textarea field. + * Ignored when the component is disabled, readonly, or invalid. + * @defaultValue undefined */ @Prop({ reflect: true }) highlight?: boolean;libs/core/src/components/pds-textarea/test/pds-textarea.spec.tsx (1)
984-1037: Nice highlight spec coverage. Add a couple more cases.
- Precedence: add tests for readonly and invalid.
- Parsing: add a case that highlight="false" does not set the attribute.
describe('highlight', () => { @@ it('semantic states take precedence over highlight', async () => { @@ expect(textarea?.hasAttribute('disabled')).toBe(true); }); + + it('ignores highlight when readonly', async () => { + const { root } = await newSpecPage({ + components: [PdsTextarea], + html: `<pds-textarea component-id="ta-ro" highlight readonly value="x"></pds-textarea>`, + }); + expect(root?.hasAttribute('highlight')).toBe(true); + const textarea = root?.shadowRoot?.querySelector('textarea'); + expect(textarea?.readOnly).toBe(true); + }); + + it('ignores highlight when invalid', async () => { + const { root } = await newSpecPage({ + components: [PdsTextarea], + html: `<pds-textarea component-id="ta-invalid" highlight invalid error-message="err" value="x"></pds-textarea>`, + }); + expect(root?.hasAttribute('highlight')).toBe(true); + const field = root?.shadowRoot?.querySelector('.pds-textarea__field'); + expect(field?.classList.contains('is-invalid')).toBe(true); + }); + + it('treats highlight="false" as false', async () => { + const { root } = await newSpecPage({ + components: [PdsTextarea], + html: `<pds-textarea component-id="ta-false" highlight="false"></pds-textarea>`, + }); + expect(root?.hasAttribute('highlight')).toBe(false); + }); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
libs/core/src/components.d.ts(58 hunks)libs/core/src/components/pds-textarea/docs/pds-textarea.mdx(1 hunks)libs/core/src/components/pds-textarea/pds-textarea.scss(1 hunks)libs/core/src/components/pds-textarea/pds-textarea.tsx(1 hunks)libs/core/src/components/pds-textarea/readme.md(1 hunks)libs/core/src/components/pds-textarea/stories/pds-textarea.stories.js(5 hunks)libs/core/src/components/pds-textarea/test/pds-textarea.e2e.ts(1 hunks)libs/core/src/components/pds-textarea/test/pds-textarea.spec.tsx(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Cursor Bugbot
- GitHub Check: Cycode: Vulnerable Dependencies
- GitHub Check: Cycode: Vulnerable Dependencies
🔇 Additional comments (8)
libs/core/src/components/pds-textarea/test/pds-textarea.spec.tsx (1)
665-665: LGTM on removing explicit false.Relying on absence of hide-label is cleaner and matches Stencil boolean handling.
libs/core/src/components/pds-textarea/stories/pds-textarea.stories.js (5)
15-15: LGTM!The
highlightprop is correctly added to the default args with an appropriate default value offalse.
49-49: LGTM!The property binding syntax is correct for Lit. Using
.highlight(dot prefix) ensures the value is set as a property rather than an attribute, which is the recommended approach for boolean values.
114-121: LGTM!The new
Highlightstory effectively demonstrates the highlight feature with a clear example. The pre-populated value helps visualize how the highlighted state appears with content.
158-158: LGTM!The highlight prop is correctly propagated to the
withActionLinkstory template, ensuring consistency across all story variations.
183-183: LGTM!The highlight prop is correctly propagated to the
withActionButtonstory template, maintaining consistency across all story variations.libs/core/src/components.d.ts (2)
1831-1834: LGTM!The
highlightproperty is correctly added to theComponents.PdsTextareainterface with appropriate typing and documentation. The optional marker (?) correctly indicates this is a non-required prop.
4522-4525: LGTM!The
highlightproperty is correctly added to theLocalJSX.PdsTextareainterface, maintaining consistency with the Components namespace definition. This enables proper TypeScript support when using the component in JSX.
Description
Adds a
highlightboolean prop topds-textareathat applies purple accent styling to draw attention to important fields.What it does:
Why:
Provides a visual way to emphasize important textarea fields without overriding critical user feedback states.
Fixes https://kajabi.atlassian.net/browse/DSS-1562
Type of change
How Has This Been Tested?
Test coverage:
reflect: true)Test Configuration:
Checklist:
If not applicable, leave options unchecked.