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

Fix: Lots of properties are not working in fluent-design-system-provider #528

Merged
merged 2 commits into from
Jul 13, 2023

Conversation

LuohuaRain
Copy link
Contributor

Pull Request

📖 Description

The units for these properties were incorrect and could not be applied even when values were set, due to the absence of measurement units: px. Therefore, it was decided to change their data type to string, allowing users to determine both the value and unit type as needed, while also maintaining consistency with the official FluentUI property types (ref: https://github.com/microsoft/fluentui/blob/57de08c18915440cdc54a1242bbdf948e2b0f562/packages/web-components/src/design-system-provider/index.ts#L426).

For example, if the TypeRampMinus1FontSize property was assigned a value of float 12, but did not include a unit such as "px" or "em", the value would not be applied correctly in the user interface. By changing the data type to string, users can now include the appropriate unit of measurement along with the value, providing greater flexibility in how the properties can be used.

🎫 Issues

👩‍💻 Reviewer Notes

📑 Test Plan

✅ Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

Component-specific

  • I have added a new component
  • I have modified an existing component

⏭ Next Steps

@LuohuaRain
Copy link
Contributor Author

@microsoft-github-policy-service agree

@vnbaaij vnbaaij self-requested a review July 13, 2023 09:54
@vnbaaij vnbaaij merged commit 2ffef40 into microsoft:main Jul 13, 2023
4 checks passed
@LuohuaRain LuohuaRain deleted the patch-1 branch September 27, 2023 06:27
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.

3 participants