Making Group and Summary regular components, only node passed to components #987
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 8 commits
March 9, 2023 16:44
… means overrides might disappear, which might break Likert)
…w that the node is passed into a component from GenericComponent (and overridden props are not passed along)
… to the component so they can be handled there instead of using a Context (a context did not affect the directRender() check, so GenericComponent also rendered errors, and the <tbody> got children that were not <tr> elements, because GenericComponent wrapped them).
…need for renderLayoutNode(), which was just a component-switcher _outside_ of GenericComponent, so there was no reason a Summary or Group could not just be rendered in the same way as other components.
…irly safely assume that the function will always return an object (otherwise GenericComponent will still refuse to render anything, which we also have unit tests for).
…can render itself in a Summary
olemartinorg
added
the
kind/other
added 3 commits
March 10, 2023 10:53
…y. It doesn't have to be this complex, as you can't really click a button that does not exist (and is visible), so cypress would fail the click anyway. Focussing it first is also pointless. Simplifying the code, hoping that will avoid sporadic failures where the elements disappear before actions can be performed.
…borked it, but the cypress tests caught that nicely. Passing each prop on its own is cleaner and works better.
…DOM changed at some point here, so the wrapped items were updated and confuses cypress when the button is clicked. This fixes the problem.
framitdavid
approved these changes
Mar 10, 2023
| label={label} | ||
| overrides={overrides} | ||
| /> | ||
| {RenderSummary && component instanceof FormComponent ? ( |
There was a problem hiding this comment.
I think we can just check if RenderSummary is not null. Because RenderSummary is set to null if the component is not an instance of FormComponent at line 116. :D
Suggested change
| {RenderSummary && component instanceof FormComponent ? ( | |
| {RenderSummary ? ( |
There was a problem hiding this comment.
Yup, but typescript doesn't agree with that.. 🤷 It complains because it didn't infer that component must be a FormComponent if RenderSummary is set, so the call to component.renderSummaryBoilerplate() below fails without that check.
Co-authored-by: David Øvrelid <46874830+framitdavid@users.noreply.github.com>
Co-authored-by: David Øvrelid <46874830+framitdavid@users.noreply.github.com>
|
SonarCloud Quality Gate failed. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Yet another installment after #976 made
GenericComponentpossible render with just thenodeobject as input. This time:nodefromGenericComponent, meaning this PR takes this concept one step further. Components no longer should require every component property as input when rendering themselves. A lot of tests had to be rewritten to make this work.GroupandSummaryhas been promoted (or demoted?) to regular components, which render viaGenericComponent. No more need for a super-function that decides if a component can be rendered usingGenericComponentor directly rendered (any component can now ask to be directly rendered).SummaryComponentSwitchanymore, as every component type can now implement how it is to be rendered in a Summary instead of the Summary component having to know about different component types.Related Issue(s)
Verification/QA
src/layout/layout.d.tsandlayout.schema.v1.json, and these are all backwards-compatiblekind/*label to this PR for proper release notes grouping