Skip to content
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

Form section fix #169

Merged
merged 2 commits into from
Jan 2, 2025
Merged

Form section fix #169

merged 2 commits into from
Jan 2, 2025

Conversation

skrobek
Copy link
Collaborator

@skrobek skrobek commented Jan 2, 2025

PR Type

Enhancement, Tests


Description

  • Added isRequired prop to FormSection component for required field indication.

  • Updated FormSection to display a red asterisk for required fields.

  • Added new tests to validate isRequired prop behavior in FormSection.

  • Updated CSS with new margin-left utility classes and bumped package version.


Changes walkthrough 📝

Relevant files
Enhancement
style.css
Added new margin-left utility classes                                       

style.css

  • Added new utility classes .ml-1 and .ml-0.5 for margin-left spacing.
  • +8/-0     
    FormSection.tsx
    Added `isRequired` prop to FormSection component                 

    src/components/ui/form/form-section/FormSection.tsx

  • Introduced isRequired prop to FormSection component.
  • Displayed a red asterisk when isRequired is true.
  • +6/-2     
    Tests
    FormSection.spec.tsx
    Added tests for `isRequired` prop in FormSection                 

    src/components/ui/form/form-section/FormSection.spec.tsx

  • Added tests for isRequired prop in FormSection.
  • Verified rendering of required asterisk based on isRequired value.
  • +10/-0   
    Configuration changes
    package.json
    Updated package version                                                                   

    package.json

    • Bumped package version from 0.12.23 to 0.12.24.
    +1/-1     

    💡 PR-Agent usage: Comment /help "your question" on any pull request to receive relevant information

    Copy link

    github-actions bot commented Jan 2, 2025

    PR Reviewer Guide 🔍

    Here are some key observations to aid the review process:

    ⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
    🧪 PR contains tests
    🔒 No security concerns identified
    ⚡ Recommended focus areas for review

    Possible Accessibility Concern

    The red asterisk for required fields is added using the text-red-500 class. Ensure that this visual indicator is accompanied by an accessible label or ARIA attribute for screen readers.

    export const FormSection: FC<Props> = ({ children, title, hint = null, isRequired = false }) => {
      return (
        <div className='py-6 flex flex-col gap-2'>
          <div className='flex flex-col gap-1'>
            <h5 className='text-lg font-medium leading-5 text-neutral-dark-800'>
              {title}
              {isRequired && <span className='text-red-500 ml-0.5'>*</span>}
            </h5>

    Copy link

    github-actions bot commented Jan 2, 2025

    PR Code Suggestions ✨

    Explore these optional code suggestions:

    CategorySuggestion                                                                                                                                    Score
    Possible issue
    Validate the isRequired prop to ensure it is treated as a boolean value

    Ensure that the isRequired prop is properly validated to avoid potential runtime
    errors when non-boolean values are passed.

    src/components/ui/form/form-section/FormSection.tsx [10-21]

     export const FormSection: FC<Props> = ({ children, title, hint = null, isRequired = false }) => {
       return (
         <div className='py-6 flex flex-col gap-2'>
           <div className='flex flex-col gap-1'>
             <h5 className='text-lg font-medium leading-5 text-neutral-dark-800'>
               {title}
    -          {isRequired && <span className='text-red-500 ml-0.5'>*</span>}
    +          {Boolean(isRequired) && <span className='text-red-500 ml-0.5'>*</span>}
             </h5>
             {hint !== null && <div className='text-gray-500'>{hint}</div>}
           </div>
           <div className='mt-2'>{children}</div>
         </div>
       );
     };
    Suggestion importance[1-10]: 7

    Why: The suggestion ensures that the isRequired prop is explicitly treated as a boolean, which can help prevent potential runtime errors if non-boolean values are passed. While the current implementation is functional, this change improves robustness and aligns with best practices for prop validation.

    7

    @skrobek skrobek merged commit a6124dc into main Jan 2, 2025
    2 checks passed
    @skrobek skrobek deleted the form-section-fix branch January 2, 2025 16:15
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant