-
-
Notifications
You must be signed in to change notification settings - Fork 915
Add support for more inline styles #544
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 support for more inline styles #544
Conversation
lib/src/css_parser.dart
Outdated
/// List<css.LiteralTerm> might include other values than the ones we want for [BorderSide.width], so make sure to remove those before passing it to [ExpressionMapping] | ||
borderWidths.removeWhere((element) => element.text != "thin" && element.text != "medium" && element.text != "thick" && !(element is css.LengthTerm)); | ||
List<css.Expression> borderColors = value.where((element) => ExpressionMapping.expressionToColor(element) != null).toList(); | ||
List<css.LiteralTerm> temp = value.whereType<css.LiteralTerm>().toList(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a huge of generic variable names such as temp
. But I guess it works fine.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree, I copy/pasted off of the text-decoration code and it had temp. Changed it :)
@@ -418,4 +539,29 @@ class ExpressionMapping { | |||
return null; | |||
} | |||
} | |||
|
|||
static Color hslToRgbToColor(String text) { | |||
final hslText = text.replaceAll(')', '').replaceAll(' ', ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's happening here? At the very least this needs some comment with an example. But really this should have some unit tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HSL Text input: hsl(39, 100%, 50%)
CSS Parser package output: 39, 100%, 50%)
The first line removes the extra spaces and the trailing ) so the values can be extrapolated into a list. We do the exact same thing for the rgbaToColor
function.
lib/src/css_parser.dart
Outdated
case "thick": | ||
return 6.0; | ||
default: | ||
return double.tryParse(value.text.replaceAll(new RegExp(r'\s+(\d+\.\d+)\s+'), '')); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you are safe-guarding again non-digit characters here, but you are just stripping them... I think we should discard any input altogether instead if it is not a number. For example, we don't want the input l33t
to parse as 33
I think, but just not 'work'.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hm this is tricky, I agree but at the same time the only other solution is to return null
, in which case the border width becomes the default set by flutter. I would hope that people are inputting well-formed HTML, since obviously border-width
only accepts numbers and those three terms. I did make this change, I'll leave it up to you to decide the best course of action.
Update: Actually I implemented support for em
, rem
, px
, % terms
, etc. They do not currently inherit parent (if they have a parent with a border), instead any relative values are calculated off of Flutter's default border width of 1.0.
lib/src/utils.dart
Outdated
Map<String, String> namedColors = { | ||
"White": "#FFFFFF", | ||
"Silver": "#C0C0C0", | ||
"Gray": "#808080", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if you tried to align the hex codes, but it looks weird now - I would not even bother with this at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Really odd... There were \t (tab) characters lurking in there but in Android Studio it just showed as a space ?? This alignment weirdness should be fixed now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job.
…into feature/more-inline-styles � Conflicts: � lib/src/css_parser.dart � lib/src/utils.dart
…h text decoration
@erickok this is ready to go and merged with nullsafety. It includes a fix for a red-screen error I encountered with text decoration. |
Add support for the
border
inline style, along with hsl, hsla, and named colors (for now the only named colors supported are the legacy named colors - more can be added quite easily, the list would be very long though).While testing this I came across a quirk of the library:
It seems that the container that holds text doesn't size itself to the size of the text, so borders are going to span the full width of the screen. I investigated this and it is a result of the
StyledText
class using aSizedBox
with widthdouble.infinity
when rendering text. I removed theSizedBox
but the behavior didn't change. I don't know if this is fixable, we can't useIntrinsicWidth
because of the rich text widget.The first two commits are mistakes on my part and the changes cancel each other out, so they can be ignored.