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 support for enum using OneOf constructs #1591

Merged
merged 5 commits into from
Jun 19, 2020
Merged

Add support for enum using OneOf constructs #1591

merged 5 commits into from
Jun 19, 2020

Conversation

eneufeld
Copy link
Member

  • Add an example for enums
  • Add a tester for oneOf enums
  • Add a control to render oneOf enums based on MUISelect

Signed-off-by: Eugen Neufeld eneufeld@eclipsesource.com

@eneufeld
Add support for enum using OneOf constructs
5fb257a 
- Add an example for enums
- Add a tester for oneOf enums
- Add a control to render oneOf enums based on MUISelect

Signed-off-by: Eugen Neufeld <eneufeld@eclipsesource.com>
@eneufeld eneufeld requested a review from sdirix June 17, 2020 12:38
sdirix
sdirix requested changes Jun 17, 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 good, just needs some polish.

packages/core/src/util/cell.ts Outdated Show resolved Hide resolved
packages/core/src/util/renderer.ts Show resolved Hide resolved
packages/core/src/util/renderer.ts Outdated Show resolved Hide resolved
packages/core/src/util/renderer.ts Outdated Show resolved Hide resolved
packages/core/src/util/renderer.ts Outdated Show resolved Hide resolved
packages/core/src/util/renderer.ts Outdated Show resolved Hide resolved
packages/vanilla/src/cells/EnumCell.tsx Outdated Show resolved Hide resolved
@eneufeld
Incorporate code review
0ebbd4b 
- Updated dependencies to include typescript > 3.7
- use cell map function in HOC
@coveralls
Copy link

coveralls commented Jun 17, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.3%) to 88.961% when pulling ffa72ee on eneufeld:feat/enumOneOf into 8e0735b on eclipsesource:master.

@eneufeld eneufeld requested a review from sdirix June 17, 2020 19:25
@sdirix sdirix added this to the 2.4.1 milestone Jun 18, 2020
sdirix
sdirix requested changes Jun 18, 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.

Only minor items left

packages/core/src/util/cell.ts Outdated Show resolved Hide resolved
packages/core/src/util/renderer.ts Outdated Show resolved Hide resolved
packages/core/src/util/renderer.ts Outdated Show resolved Hide resolved
@eneufeld eneufeld requested a review from sdirix June 19, 2020 10:32
sdirix
sdirix requested changes Jun 19, 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.

Something is broken. The titles are no longer used in the one-of-enum.

@eneufeld @sdirix
Update packages/core/src/util/renderer.ts
ffa72ee 
Co-authored-by: Stefan Dirix <sdirix@eclipsesource.com>
sdirix
sdirix approved these changes Jun 19, 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.

Works fine now!

@sdirix sdirix merged commit 432af77 into eclipsesource:master Jun 19, 2020
This was referenced Jun 19, 2020
Closed
Closed
@sblack-usu sblack-usu mentioned this pull request Mar 8, 2022
Merged
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
None yet
Projects
None yet
Milestone
2.4.1
Development

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