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

Bugfix/fix input example #256

Merged
merged 4 commits into from
Jan 25, 2019
Merged

Conversation

akinsho
Copy link
Member

@akinsho akinsho commented Jan 24, 2019

This fixes the input component example which #205 broke, it involves having to pull values out of the style list which I wanted to create a sort of getter for but having done a bit of reading and wrangling with the compiler doesn't seem straight forward, typing gets a little tricky with variants will keep it in mind and revisit once I've leveled up my reason/ocaml know-how (tracked by #239)

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 🤔

@@ -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

@akinsho akinsho merged commit f4994ab into revery-ui:master Jan 25, 2019
@akinsho akinsho deleted the bugfix/fix-input branch January 25, 2019 08:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants