Skip to content

Conversation

@CodeMaverick2
Copy link
Contributor

@CodeMaverick2 CodeMaverick2 commented Jul 25, 2024

Issue -> #8268

Fixes #8268

Description

This PR adds the prototypeName as a CSS class to the SVG group element of each block in the BlockSvg constructor. This enhancement allows for easier styling and identification of blocks based on their prototype.

Changes

  • Modified the BlockSvg constructor to add the prototypeName as a CSS class to this.svgGroup_ using dom.addClass().

Benefits

  • Improved block identification in the DOM
  • Enhanced styling capabilities for specific block types

This change is backwards-compatible and provides a hook for future enhancements or debugging.

@CodeMaverick2 CodeMaverick2 requested a review from a team as a code owner July 25, 2024 10:05
@CodeMaverick2 CodeMaverick2 requested a review from cpcallen July 25, 2024 10:05
@BeksOmega BeksOmega assigned BeksOmega and unassigned cpcallen Jul 30, 2024
@BeksOmega BeksOmega requested review from BeksOmega and removed request for cpcallen July 30, 2024 02:21
Copy link
Contributor

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

Based on the other PR for this issue, I think this is going to fail some unit tests when the prototypeName doesn't exist. Could you wrap this in a check that it exists before trying to apply it?

@CodeMaverick2 CodeMaverick2 force-pushed the issue3 branch 2 times, most recently from ddb69b6 to d5faba0 Compare July 30, 2024 12:20
@BeksOmega BeksOmega changed the title Added the block's type as a CSS class to the block's root SVG feat: add the block's type as a CSS class to the block's root SVG Jul 30, 2024
@github-actions github-actions bot added the PR: feature Adds a feature label Jul 30, 2024
Copy link
Contributor

@BeksOmega BeksOmega left a comment

Choose a reason for hiding this comment

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

Looks great! Once this passes CI I'll get it merged =)

@CodeMaverick2
Copy link
Contributor Author

@BeksOmega Sure, If any changes needed do tell me

@BeksOmega BeksOmega merged commit 203e422 into RaspberryPiFoundation:rc/v12.0.0 Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

PR: feature Adds a feature

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants