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

Use the prop! macro for all style properties #128

Merged
merged 1 commit into from
Oct 31, 2023
Merged

Conversation

Zoxc
Copy link
Contributor

@Zoxc Zoxc commented Oct 31, 2023

This replaces the builtin style field properties with properties defined by the proc! macro. This makes them work with the hover method and animations (for some types).

This is based on top of #127.

@codecov
Copy link

codecov bot commented Oct 31, 2023

Codecov Report

Merging #128 (fea362c) into main (e0015a2) will decrease coverage by 0.02%.
The diff coverage is 11.93%.

@@           Coverage Diff            @@
##            main    #128      +/-   ##
========================================
- Coverage   3.20%   3.18%   -0.02%     
========================================
  Files         54      54              
  Lines       9684    9764      +80     
========================================
+ Hits         310     311       +1     
- Misses      9374    9453      +79     
Files Coverage Δ
src/views/img.rs 0.00% <0.00%> (ø)
src/views/svg.rs 0.00% <0.00%> (ø)
src/views/clip.rs 0.00% <0.00%> (ø)
src/animate/animation.rs 0.00% <0.00%> (ø)
src/views/rich_text.rs 0.00% <0.00%> (ø)
src/views/tab.rs 0.00% <0.00%> (ø)
src/animate/anim_val.rs 0.00% <0.00%> (ø)
src/window_handle.rs 0.00% <0.00%> (ø)
src/animate/anim_id.rs 0.00% <0.00%> (ø)
src/views/label.rs 0.00% <0.00%> (ø)
... and 6 more

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

}
}
#[derive(Debug, Clone)]
pub struct Style {
Copy link
Contributor

@dzhou121 dzhou121 Oct 31, 2023

Choose a reason for hiding this comment

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

I'm wondering if it's better to move StyleMap fields here. e.g.

pub struct Style {
    pub(crate) map: HashMap<StylePropRef, StyleMapValue<Rc<dyn Any>>>,
    pub(crate) selectors: HashMap<StyleSelector, StyleMap>,
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There's quite a bit of stuff that can be refactored with this PR. I feel like the PR is sufficiently large already though.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah we can merge this first and work on stuff in another PR.

@dzhou121
Copy link
Contributor

don't know why this has conflicts after merging #127

@Zoxc
Copy link
Contributor Author

Zoxc commented Oct 31, 2023

GitHub's modifying the commits when you merge, so it didn't skip the commit for #127.

@dzhou121 dzhou121 merged commit 6f39d64 into lapce:main Oct 31, 2023
9 checks passed
@Zoxc Zoxc deleted the use-prop branch October 31, 2023 15:09
jrmoulton pushed a commit to jrmoulton/floem that referenced this pull request Nov 6, 2023
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