Skip to content
This repository has been archived by the owner on Feb 25, 2024. It is now read-only.

feat: added no splash factory #395

Merged
merged 4 commits into from
Sep 25, 2023
Merged

Conversation

TheLiux
Copy link
Contributor

@TheLiux TheLiux commented Sep 4, 2023

Removed Material splash effect on widgets, to align Yaru library with Canonical Vanilla Design, this PR closes #382

Before Now
https://github.com/ubuntu/yaru.dart/assets/62995808/2c658eec-800b-4c2a-ad6c-446418bbfb4e https://github.com/ubuntu/yaru.dart/assets/62995808/1779cb61-e072-4216-9395-d7a2f98dab59

@Feichtmeier
Copy link
Member

hi @TheLiux thanks for you PR

I am not sure about this one honestly

Testing this in this branch https://github.com/ubuntu/yaru_widgets.dart/tree/test_no_splash
in yaru widgets

@Jupi007 @madsrh @d-loose @elioqoshi please test this as well when you have time
I kinda really like the ripple effect but I am not alone

@Jupi007
Copy link
Member

Jupi007 commented Sep 4, 2023

Honestly, I have mixed feelings. 🤔
I like this effect too.
But after test the yaru_widgets branch, I agree that it feels more native and less "mobile".

@TheLiux
Copy link
Contributor Author

TheLiux commented Sep 5, 2023

I think that without ripple the effect is less material-like and more yaru-like.

If someone want the ripple and someone no, maybe a more effective approach would be to implement a bool that enables or disables the ripple effect, allowing users to choose whether they want it or not.

@Jupi007
Copy link
Member

Jupi007 commented Sep 5, 2023

If someone want the ripple and someone no, maybe a more effective approach would be to implement a bool that enables or disables the ripple effect, allowing users to choose whether they want it or not.

The idea is good, but it would be useless because you can already use copyWith to change that kind of thing 🙂

@TheLiux
Copy link
Contributor Author

TheLiux commented Sep 5, 2023

If someone want the ripple and someone no, maybe a more effective approach would be to implement a bool that enables or disables the ripple effect, allowing users to choose whether they want it or not.

The idea is good, but it would be useless because you can already use copyWith to change that kind of thing 🙂

Yeah, that's fair, i didn't think that 😄

@elioqoshi
Copy link
Contributor

Sorry I am late here as I was out of office.

First of all, thanks for the contribution @TheLiux !
While we want to unify Canonical Design, Ubuntu is in a bit of a special place and Ubuntu Desktop has some other requirements than some other highly technical enterprise Canonical products. My take on that is that Yaru should look and feel like GTK4, with Ubuntu brand elements on top. I have no strong opinion on the ripple effect (apart that it is a bit sluggish on lower end hardware) but in order to remove something, I'd like us to have a good story or argument why that would be the right thing, otherwise we could act prematurely.

@TheLiux
Copy link
Contributor Author

TheLiux commented Sep 25, 2023

Hi @elioqoshi !

Honestly, like I mentioned earlier, the ripple effect in this reminds me a lot of Material Design. If someone uses a Flutter app and compares it to a GTK app, they're bound to see some differences. Even though this package is built on top of the Flutter Material Library, I think the main aim here should be to make it look less like Material and more like Yaru.

So, I understand that there will always be some differences between native apps and Flutter apps. But if you come across a noticeable difference, why not eliminate it to maintain a native feel?

@elioqoshi
Copy link
Contributor

I agree with that thinking @TheLiux. We should have a conversation exactly like that and go from there with an implementation once we found a consensus. @Jupi007 @Feichtmeier can you remind me what's the reasoning behind having the ripple effect? If it's not in GTK I can understand the thinking that we might want to reevaluate that. What do you think?

@Jupi007
Copy link
Member

Jupi007 commented Sep 25, 2023

@elioqoshi material design comes from mobile, so the ink ripple can be considered as a tactile tap response.
Honestly, even if I like this effect, I agree that remove it enforce the native feeling.

@Feichtmeier
Copy link
Member

How about we'd keep it if yaru.dart is used in a mobile device? In common_themes there is a flag for this

@elioqoshi
Copy link
Contributor

@Feichtmeier That seems reasonable to me.

@Jupi007
Copy link
Member

Jupi007 commented Sep 25, 2023

@Feichtmeier this is a good idea 👍

lib/src/themes/common_themes.dart Outdated Show resolved Hide resolved
@Feichtmeier Feichtmeier self-requested a review September 25, 2023 16:34
@Feichtmeier Feichtmeier enabled auto-merge (squash) September 25, 2023 16:36
@Feichtmeier Feichtmeier merged commit dfc4fcb into ubuntu:main Sep 25, 2023
6 checks passed
@github-actions github-actions bot mentioned this pull request Sep 25, 2023
@Feichtmeier
Copy link
Member

Result is already visible in https://ubuntu.github.io/yaru.dart/#/
if your browser still shows the old page you need to clear the cache
@elioqoshi

@TheLiux TheLiux deleted the feat/no-splashfactory branch September 26, 2023 07:48
TheLiux added a commit to TheLiux/yaru.dart that referenced this pull request Nov 14, 2023
* feat: added no splash factory

* Update lib/src/themes/common_themes.dart

* fix fail

---------

Co-authored-by: Stefano Liuzzo Scorpo <stefano.liuzzo@devmy.it>
Co-authored-by: Frederik Feichtmeier <frederik.feichtmeier@gmail.com>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

remove splash effect from yaru
4 participants