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

Add examples of Card.outlined and Card.filled in Playground #224

Open
albemala opened this issue Mar 11, 2024 · 3 comments
Open

Add examples of Card.outlined and Card.filled in Playground #224

albemala opened this issue Mar 11, 2024 · 3 comments
Assignees
Labels
enhancement New feature or request samples Issue concerns the examples related to the package, but not the Themes Playground Themes Playground V8 This issue concerns version 8 and will be addressed in it
Milestone

Comments

@albemala
Copy link

I see there are these kind of cards displayed in the Theme simulator, but they are missing from the "Widgets showcase" and "Card" sections. It might be nice to have a few examples there as well.

@rydmike
Copy link
Owner

rydmike commented Mar 12, 2024

Hi @albemala,

Yes that is a good suggestion, they should be there as well. I actually had to check, because I was not sure if I might had added them already in a not yet released dev branch, but nope. I had not. Definitely will add them.

BTW, when you mentioned this I tested the new Card.filled and Card.outlined consructors a bit and noticed another sad theming limitation in Flutter SDK.

If you theme border radius on Card to another radius than the default one, you loose the outline on the Card.outline constructor. Because as with many other widgets they do internal variant magic that only works when the component theme is null and you cannot give the outlined variant another theme that would include the outline in its theme, while the elevated (default) and filled one does not have one.

I will file a Flutter issue about this and I also need to put a warning into the Playground about this. The issue is similar to the FilledButton and FilledButton.tonal colors theming case and FAB shape too for different sized FABs.

@rydmike rydmike self-assigned this Mar 12, 2024
@rydmike rydmike added enhancement New feature or request samples Issue concerns the examples related to the package, but not the Themes Playground Themes Playground V7 This issue concerns version 7 and will be addressed in it labels Mar 12, 2024
@rydmike rydmike added this to the 7.4.0 milestone Mar 12, 2024
rydmike added a commit that referenced this issue Mar 12, 2024
@rydmike
Copy link
Owner

rydmike commented Mar 12, 2024

This presentation is now there, coming in version 7.4.0.

I also used it on the Card theme settings page. Just sniped the code from the theme simulator, they now use the same code of course.

Screen.Recording.2024-03-12.at.13.41.08.mov

I also show a component styled version of the Card.outlined and a themed version of it.

This shows how a none default radius, due to the variant theming issue, kills the outlined border on themed Card.outlined variant, since there are no variant themes to support an own theme for it.

I also added a special case to FCS card theme, so it does not create a theme for themed card shape when null or default radius value is used. This way, for that specific case, FCS theming will not "kill" the border by creating a custom shape in the the card theme when M3 default value is used. In current version it does that. So that fix/improvement was also added and tests updated to work with the mod.

This way the impact with FCS made card theme is exactly the same as it is in vanilla Flutter ThemeData and its Card theme. Nothing I can do about the SDK Card theming limitation, other than file the theming issue. There are already MANY other similar variant theming issues. I now have two mor of the to file, when including this one. I'll try to file both later today.

After filing the issues, I still also need to put one of those "known" Flutter theming issues info icons in the Playground and FCS docs for both of them.

The other one is about full screen mode and search bar window getting shape in fullscreen mode, if you want to give the popup version another radius that the default one.

I have no idea why trivial issues like this keep creeping into the SDK and its theming. They do test, and a lot. But maybe they also need to visually try different configs in test benches to actually see what happens in different use cases, to know what to cover with the tests.


Also noticed:

The code in the theme simulator that shows and uses the official M3 demo app, is from an old fork and mod I made to be able to add into the Playground. At the time I sniped the code, the Card presentation did no use the actual Card.filled and Card.outlined widgets. They used styled widgets to make M3 Card variants. Since at that time the variant constructors for Card did not exist in Flutter SDK.

I noticed that the official M3 demo at that point used the ColorScheme.outline color to set the color and in the styled Card widget they made to match the M3 outlined Card style. That color is actually incorrect M3 spec, it should be ColorScheme.outlineVariant. M3 spec says and the default one created in Flutter when you don't have a shape theme, also now uses that. So if the Flutter M3 app has been updated to use the newer Card constructors, then the wrong color in it has also been fixed, otherwise maybe not. I should go and check its status. I landed some other minor fixes in the official M3 demo app before.

I do clearly recall that in a original M3 spec and when the first versions of the Flutter M3 demo app was made, the Card outline M3 spec did use the outline color, so it was not an error then, but now spec is outlineVariant. I remember because I thought back then, that it would have looked better with the less prominent outlineVariant color. Seems like someone on Material theme thought so too and spec has been changed 😄

@albemala
Copy link
Author

Thanks @rydmike for the prompt response and the addition to the Playground!

It's unfortunate that you discovered this theming limitation, but on the bright side, now the Flutter team (or someone else) can work on a fix. Hopefully also on all the other similar issues you've mentioned. Maybe there is a common solution for all of them.

I see M3 specs are being changed constantly, and it's not easy to keep up with all of them!

I look forward to see this fix landing in the next stable package release, again thanks for working on this!

@rydmike rydmike added V8 This issue concerns version 8 and will be addressed in it and removed V7 This issue concerns version 7 and will be addressed in it labels Apr 26, 2024
@rydmike rydmike modified the milestones: 7.4.0, 8.0.0 Apr 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request samples Issue concerns the examples related to the package, but not the Themes Playground Themes Playground V8 This issue concerns version 8 and will be addressed in it
Projects
Status: Done
Development

No branches or pull requests

2 participants