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.

Sign up for GitHub

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

Bugfix/fix input example #256

Merged
merged 4 commits into from
Jan 25, 2019
Merged

Bugfix/fix input example #256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e169eec
[WIP] Add unwrap functionality
akinsho Jan 23, 2019
d7f7909
Add getter for fontsize
akinsho Jan 23, 2019
bc50f9c
Add type annotation to unwrap function
akinsho Jan 23, 2019
7fc36d0
Add a style extraction function and fix input with it
akinsho Jan 24, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 18 additions & 5 deletions src/UI/Style.re
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,7 @@ type xy = {
vertical: int,
};

type props = [
type coreStyleProps = [
| `FlexGrow(int)
| `FlexDirection(LayoutTypes.flexDirection)
| `JustifyContent(LayoutTypes.justify)
Expand Down Expand Up @@ -248,9 +248,11 @@ type fontProps = [ | `FontFamily(string) | `FontSize(int)];
these nodes are typed to only allow styles to be specified
which are relevant to each
*/
type textStyleProps = [ fontProps | props];
type viewStyleProps = [ props];
type imageStyleProps = [ props];
type textStyleProps = [ fontProps | coreStyleProps];
type viewStyleProps = [ coreStyleProps];
type imageStyleProps = [ coreStyleProps];

type allProps = [ coreStyleProps | fontProps];

let emptyTextStyle: list(textStyleProps) = [];
let emptyViewStyle: list(viewStyleProps) = [];
Expand Down Expand Up @@ -344,12 +346,23 @@ let overflow = o => `Overflow(o);
let color = o => `Color(o);
let backgroundColor = o => `BackgroundColor(o);

/*
Helper function to narrow down a list of all possible style props to
one specific to a type of component
*/
let rec extractViewStyles = (styles: list(allProps)): list(viewStyleProps) =>
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would like to generalise this but can't figure out how, its that or create one whenever we need a utility to separate a certain subset of styles from another. Say you create a custom component comprised of text and view components so it will take a style list of coreStyleProps + extra fontProps but then as the view is not allowed to have fontProps they need to be split out in the implementation part of the custom component.

Its very possible this isn't something we might need a lot 🤷‍♂️

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question, I'm not sure this would work either 🤔

switch (styles) {
| [] => []
| [#viewStyleProps as v, ...list] => [v, ...extractViewStyles(list)]
| [_, ...list] => extractViewStyles(list)
};

/*
Apply style takes all style props and maps each to the correct style
and is used to build up the style record, which is eventually
used to apply styling to elements.
*/
let applyStyle = (style, styleRule: [< props | fontProps]) =>
let applyStyle = (style, styleRule: [< coreStyleProps | fontProps]) =>
switch (styleRule) {
| `AlignItems(alignItems) => {...style, alignItems}
| `JustifyContent(justifyContent) => {...style, justifyContent}
Expand Down
21 changes: 16 additions & 5 deletions src/UI_Components/Input.re
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ let make =
computed styles
*/

let viewStyles =
let allStyles =
Style.(
merge(
~source=[
Expand All @@ -147,14 +147,14 @@ let make =
justifyContent(`FlexStart),
overflow(LayoutTypes.Hidden),
cursor(MouseCursors.text),
...defaultStyles,
],
~target=style,
)
);

/*
TODO: convert this to a getter utility function
*/
let viewStyles = Style.extractViewStyles(allStyles);

let inputHeight =
List.fold_left(
(default, s) =>
Expand All @@ -166,6 +166,17 @@ let make =
style,
);

let inputFontSize =
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also tried generalising this as it seems like a much more common use case to get the specific value of an item in the style list if you want to do some internal logic based on it.

I tried something along the lines of the above but

  /* Inside a fold_left */
  switch(valueIWant, variantType) {
   | ("marginTop", MarginTop(mt)) => mt
  /* ....etc */
}

the compiler doesn't seem to be able to narrow down the return type this way, could have been a question type annotating it properly but I couldn't work out how

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe for some of these - it'd be worth just calling Style.applyStyle, getting a record, and asking that for the value?

Like:

let { height, fontSize, color } = Style.apply(style);

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this issue with that approach is that since style.t is strictly typed and records in general are strictly typed I can't return a record that maps to exactly what was passed in if I return a type t then I'm forced to set some default values which aren't all undefined which can muddy the waters in terms of know what was actually in the style list and just that, a potential alternative might be returning another data type like a map or a tuple, ill experiment in a separate PR

List.fold_left(
(default, s) =>
switch (s) {
| `FontSize(fs) => fs
| _ => default
},
20,
style,
);

let inputColor =
List.fold_left(
(default, s) =>
Expand All @@ -181,7 +192,7 @@ let make =
Style.[
color(hasPlaceholder ? placeholderColor : inputColor),
fontFamily("Roboto-Regular.ttf"),
fontSize(20),
fontSize(inputFontSize),
alignItems(`Center),
justifyContent(`FlexStart),
marginLeft(6),
Expand Down