Update Style class to more fully support computed property usage #2134
Conversation
Calculations based on font size should be calculated based on the current font size, not the default.
- Only set the computed property on fallback to the passed-in value if it is not blank or "inherit". - When getting a property, only set it to the default if there is no specified value - When getting a property, call the setter with the specified value if there is no computed value
- word-spacing - letter-spacing - line-height
- background - background-attachment - background-image-resolution - background-position - background-repeat - background-size
- border (color, style, width for each side) - border-radius (for each corner) - border-spacing - outline (color, style, width) This includes generic helpers utilized by the getters/setters: - _get_width - _set_border_radius_corner - _set_style_side_type
- color (and helper methods) - image-resolution - list-style (list-style-image) - page-break-after - page-break-before - transform - transform-origin - z-index
|
@simonberger I know you want to move forward with implementing a CSS parser, but this update should address a number of issues I identified in the current release. Let's plan on looking at integrating the parser for 0.8.7. |
|
The style changes which came in my mind would not replace this part. Just the parsing and the DOM matching. Maybe at some point the style class could or even should be changed but this would be another dimension. But I am completely uncertain if I want to do the project at all. Regarding this pull request I already had a quick look and I like the improved transparency of the processes inside the class it gives. I'll dive deeper some time the next days and test it with the RTL branch where the problems came up. |
|
ahhaha that's right 😅 I didn't include the text-align update since that didn't exist yet prior to you adding it to the rtl branch. But I did go ahead and work out what it needs to be. I can add it here or wait until we merge this back into your branch and update there. |
|
I noted that you didn't add a set method for text-align but thought it might still work. I reverted the text-align initial patch and restored an earlier state (just there and not in the rtl main branch yet) we discussed (#2107 (comment)) I think the final version moves the logic to set_text_align instead of set_direction but we can do it after your changes. You can surely add patches to #2107 as well. I just would not like to merge the combined branch at the end. |
Getter is excluded under the assumption that the user is hooking into the generic getter/setter, which will evaluate to the correct value if no value is set. Per the spec: a nameless value that acts as 'left' if 'direction' is 'ltr', 'right' if 'direction' is 'rtl'.
- add keyword check - add dependency on direction
|
Sorry, looks like I got ahead of myself. I thought you were happy with the state of the branch so I went ahead and merged. I made a few changes as suggested and will push them after you have a chance to review my responses to your comments. |
No description provided.