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

remove platform specific plugin configuration & allow to change text height from client code #18

Merged
merged 2 commits into from
Aug 19, 2022

Conversation

alexmercerind
Copy link
Contributor

Hey! Thanks for the awesome package for showing lyrics inside Flutter.

Only "plugins" need to have platform specific code for leveraging native functionalities through platform interface.
Since, this "package" only aims to provide non-platform specific UI only code, there is no need to create platform specific configuration with native code for each platform.
This results in unnecessary bundling of shared libraries on all platforms, which isn't even used.

More details may be found at: https://docs.flutter.dev/development/packages-and-plugins/developing-packages#types

I also removed hardcoded copyWith calls which change the rendered Text's height to 1. I don't know if this results in some problem, sure let me know.

@ozyl ozyl merged commit 4674a8f into ozyl:master Aug 19, 2022
@ozyl
Copy link
Owner

ozyl commented Aug 20, 2022

Thank you for your PR. After removing the platform configuration, the only supported platform on the pub is WEB. It was also because of this that these configurations were added before. I don't know the reason, can you help?
➡️:pub link

@alexmercerind
Copy link
Contributor Author

Thanks for letting met know!

fluent_ui also doesn't have any declared platform specific code, but on pub.dev it shows that it supports all platforms, how a package should.

I think we should add this at the end of pubspec.yaml (see: here) & remove fluttr_web_plugins from dependencies:

flutter:
  uses-material-design: true

I generally only work on plugins or non-UI FFI packages, so I didn't have much knowledge about this.

Thanks again! And sorry for breaking it.


I have raised #19 to save your time & incremented version too.

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.

None yet

2 participants