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

feat: Add a blocklyNumberField CSS class to number fields #8414

Merged
merged 2 commits into from
Jul 31, 2024

Conversation

Apocalypse96
Copy link

The basics

The details

Resolves

Fixes #8313

Proposed Changes

This PR adds a 'blocklyNumberField' CSS class to the FieldNumber element. Specifically:

  • Implements the initView method in the FieldNumber class
  • Applies the 'blocklyNumberField' class to the fieldGroup_ element
  • Adds a null check for fieldGroup_ to prevent TypeScript errors

Reason for Changes

This change allows for more specific styling of number fields in Blockly, as requested in issue #8313. It provides a way to target number fields specifically in CSS, enhancing the ability to customize the appearance of these fields.

Test Coverage

I manually tested these changes by:

  1. Creating a block with a number field
  2. Inspecting the DOM to verify that the 'blocklyNumberField' class was applied
  3. Applying CSS to the new class and verifying that it affected only number fields

Additional unit tests could be created to ensure the initView method is called and applies the class correctly.

Documentation

No documentation updates are required for this change, as it doesn't affect the public API or usage of Blockly. However, it might be beneficial to add a note in the theming documentation about the availability of this new CSS class for styling number fields.

Additional Information

This change is backwards-compatible and should not affect existing Blockly implementations. It simply adds an additional CSS class that can be used for more specific styling if desired.

@Apocalypse96 Apocalypse96 requested a review from a team as a code owner July 22, 2024 16:29
@Apocalypse96 Apocalypse96 requested a review from cpcallen July 22, 2024 16:29
@github-actions github-actions bot added the PR: feature Adds a feature label Jul 22, 2024
@BeksOmega BeksOmega requested review from BeksOmega and removed request for cpcallen July 23, 2024 15:35
@BeksOmega BeksOmega assigned BeksOmega and unassigned cpcallen Jul 23, 2024
@BeksOmega
Copy link
Collaborator

Hello, can you rebase this so that the changes are against rc/v12.0.0?

You can follow these steps:

  1. Fetch the latest changes from rc/v12.0.0:

    git fetch upstream rc/v12.0.0
  2. Checkout your branch:

    git checkout [your-branch-name]
  3. Start an interactive rebase:

    git rebase -i upstream/rc/v12.0.0
  4. In the editor that opens, identify the commits that aren't yours. These are likely the ones at the top of the list.

  5. Change the word pick to drop for each commit you want to remove.

  6. Save and close the editor.

  7. Force push your changes to your branch: Note that force pushing is a dangerous operation because it overwrites commit history, so if you dropped the wrong commits you could lose work.

    git push -f origin [your-branch-name]

Once you've done this, the PR should be on the correct branch.

Let me know if you have any questions or run into any issues!

@BeksOmega BeksOmega changed the base branch from develop to rc/v12.0.0 July 30, 2024 02:14
@github-actions github-actions bot added PR: feature Adds a feature and removed PR: feature Adds a feature labels Jul 30, 2024
@BeksOmega
Copy link
Collaborator

@Apocalypse96 Are you still interested in working on this?

@Apocalypse96
Copy link
Author

@Apocalypse96 Are you still interested in working on this?

ya sure

@BeksOmega
Copy link
Collaborator

@Apocalypse96 Are you still interested in working on this?

ya sure

In that case, could you follow the rebase instructions I posted above?

@Apocalypse96
Copy link
Author

@BeksOmega I've rebased can u review the pr

Copy link
Collaborator

@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.

This looks great thank you for the fixes! Once this passes CI I'll get it merged =)

@BeksOmega BeksOmega merged commit 8a1b015 into google:rc/v12.0.0 Jul 31, 2024
7 checks passed
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.

Add a blocklyNumberField CSS class to number fields
3 participants