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

Jump to bottom

Add style context #1652

Merged
merged 2 commits into from
Dec 3, 2020
Merged

Add style context #1652

merged 2 commits into from
Dec 3, 2020

Conversation

TheZoker
Copy link
Contributor

  • Add style context for vanilla renderers
  • Replaces core styles with the new styles context

Related issue: #1537

@sdirix sdirix added the hacktoberfest-accepted https://hacktoberfest.digitalocean.com/ label Oct 30, 2020
sdirix
sdirix requested changes Nov 12, 2020
View reviewed changes
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Looks pretty good! However we need two changes:

  • We should export the new context via the index so it can be easily used ;)
  • useStyles should contain a fallback to consume the regular context styles if there aren't any in the StyleContext

@TheZoker TheZoker requested a review from sdirix November 24, 2020 16:20
sdirix
sdirix requested changes Dec 1, 2020
View reviewed changes
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

The testcases currently fail. Can you have a look?

@TheZoker
Add style context for vanilla renderers
4210b56 
Rename style to styles


Use StyleContext instead of core styles


Rename useStyle to useStyles and call it unconditional


Add fallback, export StyleContext and fix function name


Fix reference
@TheZoker TheZoker requested a review from sdirix December 1, 2020 22:12
@coveralls
Copy link

coveralls commented Dec 2, 2020 •
edited
Loading

Coverage Status

Coverage increased (+0.04%) to 88.488% when pulling 3a526e5 on TheZoker:add-style-context into a51e373 on eclipsesource:master.

sdirix
sdirix approved these changes Dec 3, 2020
View reviewed changes
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

LGTM

@sdirix sdirix merged commit 1853fc7 into eclipsesource:master Dec 3, 2020
@TheZoker TheZoker deleted the add-style-context branch December 3, 2020 11:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sdirix sdirix sdirix approved these changes

Assignees
No one assigned
Labels
hacktoberfest-accepted https://hacktoberfest.digitalocean.com/
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@TheZoker @coveralls @sdirix