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

fix!: set buttons and textfields to be 35px high #370

Closed
wants to merge 7 commits into from

Conversation

Feichtmeier
Copy link
Member

@Feichtmeier Feichtmeier commented Jul 21, 2023

This is setting the height for buttons and togglebuttons to 35
The matching icon size seems to be 16 🤷
I adapted the example (the control view really needs some love to reduce code copy pasting at some point)

Bildschirmfoto 2023-07-21 um 11 53 30

Fixes #369

@Feichtmeier Feichtmeier requested a review from jpnurmi July 21, 2023 09:53
@Feichtmeier Feichtmeier marked this pull request as draft July 21, 2023 10:03
@Feichtmeier
Copy link
Member Author

Bildschirmfoto 2023-07-21 um 12 03 42
needs a bit more work for textfield adaption

@jpnurmi
Copy link
Member

jpnurmi commented Jul 21, 2023

Hmm, was it so that there was no clean way to control the size but you could only tweak the padding to make it coincidentally match by default? I hope this won't ruin the whole plan... :( On a related note, looks like text fields were 42px back when we were still on M2.

image

@Feichtmeier
Copy link
Member Author

you can control both the size and the content padding for the child inside

@Feichtmeier
Copy link
Member Author

Also the example s*cks because the rows try to force a specific height

I'll rework the example a bit
Then fix the textfield height

Do you have a specific time pressure on this issue? I don't know if I can finish it today

@jpnurmi
Copy link
Member

jpnurmi commented Jul 21, 2023

There's no time pressure. I just noticed some mismatching button heights (36px vs. 40px) in the installer and started wondering what was going on and where the 36px even came from...

@Feichtmeier
Copy link
Member Author

There's no time pressure. I just noticed some mismatching button heights (36px vs. 40px) in the installer and started wondering what was going on and where the 36px even came from...

ok :)
Also to be sure I better do this on buntu when I am home 👴

@Feichtmeier
Copy link
Member Author

Feichtmeier commented Jul 23, 2023

Okay slowly getting there, though it is a bit more complicated because many things affect the text field @jpnurmi

grafik

Things that need changing for this:

  • text theme body large from 16 to 14
  • input decoration theme

Things that can still totally ruin the textfields:

  • bouncing label moves the hint 2 px down -.-
  • prefix icons even with reduced icon size in the whole theme and then increased back for headerbar and navigation bars

grafik

I must admit this already looks much better. But can we live with the hint bouncing and the prefix icon alignment?
given that the prefix icon can be always adapted in for example yaru widgets ?

@Feichtmeier
Copy link
Member Author

grafik

@Feichtmeier
Copy link
Member Author

Feichtmeier commented Jul 28, 2023

@jpnurmi okay this is really weird

grafik

I found most of the things to be controllable inside the textfield, except the actual editable text ...
It can be controlled later when you instantiate a TextField though easily with fontSize
But there is no property in InputDecorationTheme that lets you control the actual fontSize of the text you can edit 🤦

What we could do is to keep those changes here because they overall look better with buttons and textfields being smaller
but then create a YaruTextField or something like this in yaur widgets forwarding every single property of textfield but then changing the font size to 14 :S

what do you suggest in general and what should be the scope of this PR?

@Feichtmeier Feichtmeier changed the title fix!: restore m2 button height -1 fix!: set buttons and textfields to be 35px high Jul 28, 2023
@jpnurmi
Copy link
Member

jpnurmi commented Jul 28, 2023

It's painful to wrap something like TextField that has tons of properties but I guess that's the only way to go until the font will be themable. :(

@Feichtmeier
Copy link
Member Author

Feichtmeier commented Jul 28, 2023

Alternative would be to expose a kYaruTextFieldFontSize = 14.0 in yaru.dart and just remember us every time we use a text field to use this constant?

or good ol inheritance?

import 'package:flutter/material.dart';

class YaruTextField extends TextField {
  const YaruTextField({super.key});

  @override
  StrutStyle? get strutStyle => ...;

  @override
  TextStyle? get style {
    return const TextStyle(fontSize: 14);
  }
}

@Feichtmeier
Copy link
Member Author

I tested this a little more and maybe we could come back to your old idea of using the VisualDensity API ? @jpnurmi
Because these sizes look super tiny when used on the web now, especially if on a mobile device

@jpnurmi
Copy link
Member

jpnurmi commented Aug 1, 2023

Or just go with the flow and accept 40px buttons 😭

@Feichtmeier
Copy link
Member Author

Or just go with the flow and accept 40px buttons 😭

Maybe we can have both. Even the current button height is too low for the weird mobile aspect ratio. Just open yaru.dart GitHub website on mobile.

@Feichtmeier
Copy link
Member Author

Feichtmeier commented Aug 8, 2023

playing around with a vanilla material3 theme and visual density.
this visual density api is more than incomplete. I would rather call it dysfunctional and confusing because basically only buttons change.
thinking about this a lot lately and also thinking about that flutter is still cross platform and we should have a way to have comfortable sizes for mobile apps with our colors. I could very well imagine someone wanting to create a mobile app with yaru.dart.
eventually we should move all size manipulations in yaru.dart to yaru_widgets and provide wrapper for buttons and textfields. then eventually we could have some kind of mapping for i.e. YaruIconButton -> isAndroid ? IconButton : YaruCompactIconButton or something

opinions @jpnurmi @Jupi007 ?

edit: also the font and icon sizes are often way too large

@Feichtmeier Feichtmeier closed this Aug 9, 2023
@Feichtmeier Feichtmeier deleted the Feichtmeier/issue369 branch August 9, 2023 17:58
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.

Better platform independant sizes
2 participants