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

Add non-semantic header option #141

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

blackfalcon
Copy link
Member

@blackfalcon blackfalcon commented Jan 6, 2025

Alaska Airlines Pull Request

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Resolves: #140

Summary:

  1. New Feature:

    • Added support for a level="none" attribute, rendering a <span> instead of a heading element (<h1>-<h6>). Default remains <h1>.
  2. Refactored Spacing Logic:

    • Consolidated spacingDecision and spacingApplied methods into a single spacingClasses getter for simplicity and reduced redundancy.
  3. Template Simplification:

    • Precomputes CSS class and style strings (spacingStyles and colorStyles) for cleaner template rendering.
  4. Test Additions:

    • New test ensures level="none" correctly renders a <span> element.

Type of change:

Please delete options that are not relevant.

  • New capability
  • Revision of an existing capability
  • Infrastructure change (automation, etc.)
  • Other (please elaborate)

Checklist:

  • My update follows the CONTRIBUTING guidelines of this project
  • I have performed a self-review of my own update

By submitting this Pull Request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

Pull Requests will be evaluated by their quality of update and whether it is consistent with the goals and values of this project. Any submission is to be considered a conversation between the submitter and the maintainers of this project and may require changes to your submission.

Thank you for your submission!

-- Auro Design System Team

Summary by Sourcery

Add support for non-semantic header elements. Refactor spacing and color logic for cleaner template rendering.

New Features:

  • Allow <auro-header> to render as a <span> element using level="none".

Tests:

  • Add tests to verify non-semantic rendering.

@blackfalcon blackfalcon requested a review from a team as a code owner January 6, 2025 23:49
Copy link

sourcery-ai bot commented Jan 6, 2025

Reviewer's Guide by Sourcery

This pull request introduces a new "level=\"none\"" option for the <auro-header> component, allowing it to render as a <span> element instead of a heading. It also refactors spacing logic for simplification and adds a corresponding test.

Class diagram for AuroHeader component changes

classDiagram
    class AuroHeader {
        +String level
        +String display
        +String color
        +String margin
        +String size
        -get spacingClasses()
        +render()
    }
    note for AuroHeader "level attribute now supports 'none'"
    note for AuroHeader "spacingClasses replaces spacingDecision and spacingApplied"
Loading

State diagram for header element rendering

stateDiagram-v2
    [*] --> CheckLevel
    CheckLevel --> Span: level='none'
    CheckLevel --> H1: level='1' or default
    CheckLevel --> H2: level='2'
    CheckLevel --> H3: level='3'
    CheckLevel --> H4: level='4'
    CheckLevel --> H5: level='5'
    CheckLevel --> H6: level='6'

    Span --> ApplyStyles
    H1 --> ApplyStyles
    H2 --> ApplyStyles
    H3 --> ApplyStyles
    H4 --> ApplyStyles
    H5 --> ApplyStyles
    H6 --> ApplyStyles

    ApplyStyles --> [*]
Loading

File-Level Changes

Change Details Files
Added support for `level=\
  • Added level=\\</li><li>now renders aelement whenlevel=\
  • Added tests to verify the new <span> rendering behavior.
src/auro-header.js
test/auro-header.test.js

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time. You can also use
    this command to specify where the summary should be inserted.

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

@blackfalcon blackfalcon self-assigned this Jan 6, 2025
@blackfalcon blackfalcon linked an issue Jan 6, 2025 that may be closed by this pull request
Copy link

github-actions bot commented Jan 6, 2025

Surge demo deployment failed! 😭

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @blackfalcon - I've reviewed your changes and they look great!

Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟡 Testing: 1 issue found
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

src/auro-header.js Outdated Show resolved Hide resolved
test/auro-header.test.js Show resolved Hide resolved
src/auro-header.js Outdated Show resolved Hide resolved
@blackfalcon
Copy link
Member Author

Attempted to compare PR to BETA, but BETA is very out of date and I was unable to fix the branch.

@blackfalcon blackfalcon force-pushed the dsande/nonsemanticheader branch 3 times, most recently from d639fc9 to 89e704f Compare January 7, 2025 00:59
This commit removes overly verbose simple functions and replaces them
with an easier to maintain getter method to generate the necessary CSS
strings that are applied to the DOM element.

Additionally the template switch statements have been refactored to
remove duplicate code and simplify maintenance.

On branch dsande/nonsemanticheader
Changes to be committed:
modified:   src/auro-header.js
This commit will add functional support for allowing users to easily
generate a visual header without the semantic context.

On branch dsande/nonsemanticheader
Your branch is up to date with 'origin/dsande/nonsemanticheader'.
Changes to be committed:
modified:   src/auro-header.js
Changes to be committed:
modified:   test/auro-header.test.js
@blackfalcon blackfalcon force-pushed the dsande/nonsemanticheader branch from 89e704f to b4722b7 Compare January 7, 2025 01:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support non-semantic header option
1 participant