Skip to content

Typography: Remove input in favor of subtitle1#10028

Merged
henon merged 2 commits intoMudBlazor:devfrom
danielchalmers:remove-input-tyopgraphy-type
Oct 15, 2024
Merged

Typography: Remove input in favor of subtitle1#10028
henon merged 2 commits intoMudBlazor:devfrom
danielchalmers:remove-input-tyopgraphy-type

Conversation

@danielchalmers
Copy link
Member

Description

Originally introduced in #8754, the input typo was created to consolidate the hardcoded styles found in _input.scss. Now looking back (and checking the material design spec) I recognize that the styles were actually subtitle1 but not hooked up to the overall typography theme provider.

I don't anticipate much backlash considering that the option was never possible before and was only recently introduced. We should maintain parity with the original spec. Now users get the best of both worlds, with the customization opportunities of the Typo property from the earlier PR, and with this going back to subtitle1 (but attached to the theme provider) a more cohesive application style that's more in tune with Material Design expectations.

I consider this a part of the overall effort in #8884,

How Has This Been Tested?

Type of Changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Documentation (fix or improvement to the website or code docs)

Checklist

  • The PR is submitted to the correct branch (dev).
  • My code follows the code style of this project.
  • I've added relevant tests.

@github-actions github-actions bot added breaking change This change will require consumer code updates PR: needs review labels Oct 15, 2024
@danielchalmers
Copy link
Member Author

Migration guide:

  • This removes Typo.input. Use Typo.subtitle1 instead.
  • This removes all input typo CSS. Use --mud-typography-subtitle1 instead.
  • This changes the order of the Typo enum

@danielchalmers danielchalmers requested a review from henon October 15, 2024 03:19
@codecov
Copy link

codecov bot commented Oct 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 91.06%. Comparing base (000fe79) to head (b678c2f).
Report is 3 commits behind head on dev.

Additional details and impacted files
@@           Coverage Diff           @@
##              dev   #10028   +/-   ##
=======================================
  Coverage   91.05%   91.06%           
=======================================
  Files         410      410           
  Lines       12522    12508   -14     
  Branches     2446     2445    -1     
=======================================
- Hits        11402    11390   -12     
  Misses        570      570           
+ Partials      550      548    -2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@henon henon merged commit 4429bc1 into MudBlazor:dev Oct 15, 2024
@henon
Copy link
Contributor

henon commented Oct 15, 2024

Added to the v8 migration guide at #9953

@ScarletKuro
Copy link
Member

ScarletKuro commented Oct 15, 2024

I don't think the migration guide is complete for this.
Since this removes Input (aka InputTypography in v8) (the removal is mentioned in the migration guide) from Theme.
Are people suppose to use Subtitle1Typography for their inputs?

@ScarletKuro
Copy link
Member

ScarletKuro commented Oct 15, 2024

  • This changes the order of the Typo enum

I would lean towards that all enums must set order explicitly, and when you remove an enum the old order stays, just the removed gets "reserved" but since it's merged already then ok. But for future it's better to do so for compatibility.

@henon
Copy link
Contributor

henon commented Oct 15, 2024

@ScarletKuro I thought the same but then this was added only recently which means the enum order changed only recently and nobody said anything, and now we are changing it back, so I didn't mention it.

@ScarletKuro
Copy link
Member

I slightly modified the guide saying that Input class and property was removed, use subtitle1, hope that's correct.

@danielchalmers
Copy link
Member Author

I slightly modified the guide saying that Input class and property was removed, use subtitle1, hope that's correct.

Yes subtitle1 is the replacement for input which is what it should have been originally @ScarletKuro

LLauter pushed a commit to cannellamedia/MudBlazor that referenced this pull request Dec 16, 2024
@thomasbach-dk
Copy link

What´s the benefit of this change? Am I missing something, or did you forget to take into account that subtitle1 could have been used for other stuff than inputs? :)

I understand that subtitle1 was originally thought to be used as input typo, but since that was never the case - until now - we have been using subtitle1 for plenty of stuff :(

@danielchalmers
Copy link
Member Author

What´s the benefit of this change? Am I missing something, or did you forget to take into account that subtitle1 could have been used for other stuff than inputs? :)

I understand that subtitle1 was originally thought to be used as input typo, but since that was never the case - until now - we have been using subtitle1 for plenty of stuff :(

Hi @thomasbach-dk! The primary goals were:

  • re-aligning with Material Design and similar libraries (never had input)
  • making the theme more consistent by requiring less setup
  • improving compatibility with apps using older versions

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change This change will require consumer code updates

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

Comments