FEAT: STYLIZE function and STYLES keyword for VID #2300
Conversation
modules/view/VID.red
Outdated
| 'else [ | ||
| either face/text [ | ||
| size: size-text face | ||
| if find [button radio check] face/type [size/x: size/x + size/y] |
There was a problem hiding this comment.
IIRC this is leftover from some older View version on which I based this PR.
modules/view/VID.red
Outdated
| rate: font: flags: options: para: data: extra: actors: draw: now?: none | ||
| ] | ||
|
|
||
|
|
There was a problem hiding this comment.
You moved opts object out of the function. Where are the object words reset to none now?
There was a problem hiding this comment.
You're right, they aren't. I have a fix in my branch (2109e7 from 2016-11-03), but it is missing from this PR. I am not very familiar with PRs, can I fix it here or should I make a new PR?
There was a problem hiding this comment.
You can fix it with an additional commit to this PR.
modules/view/VID.red
Outdated
| master (system/view/VID/styles) | ||
| styles (copy css) | ||
| true (make map! 2) | ||
| ] |
There was a problem hiding this comment.
Why using parens for actions in case instead of the more standard blocks? It confused me as I was searching for a related compose...
There was a problem hiding this comment.
I am not sure why I am using parens in case, I've been doing this for many years. I will change it (and try to rewire myself to blocks here :) )
|
I've tried to fix all things mentioned in review and also base it on latest VID version. Hopefully it is OK now. |
modules/view/VID.red
Outdated
| @@ -184,7 +176,7 @@ system/view/VID: context [ | |||
| | 'font (opts/font: make any [opts/font font!] fetch-argument obj-spec! spec) | |||
| | 'para (opts/para: make any [opts/para para!] fetch-argument obj-spec! spec) | |||
| | 'wrap (opt?: add-flag opts 'para 'wrap? yes) | |||
| | 'no-wrap (opt?: add-flag opts 'para 'wrap? no) | |||
| | 'no-wrap (add-flag opts 'para 'wrap? no opt?: yes) | |||
There was a problem hiding this comment.
Nit pick: I don't know why you did this but it doesn't this make sense?
After looking at the code for a while, it still didn't.. so I may be wrong about it..
|
@rebolek, can this be dusted off, fixed, and updated to merge? Seems like @dockimbel only had issues with some code constructs, not design. |
|
@greggirwin I need to check it against newest Red, I believe that there are some merge conflicts now. I'll take a look at it and let you know. |
No description provided.