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

Refactoring layout/node type mapping #1202

Merged
merged 13 commits into from
May 30, 2023
Merged

Conversation

olemartinorg
Copy link
Contributor

@olemartinorg olemartinorg commented May 26, 2023

Description

Fixing a few issues:

  • Our master-list of components can cause merge conflicts if forks try adding their own layout components, because we all have to maintain the master list. This change moves the list out from git, and instead auto-generates it in an ignored file (src/layout/components.ts, which is generated by running yarn gen, which should also happen automatically for all relevant operations). This should make a solution as described in Analysis: Template for app frontend #200 more feasible.
  • The type definitions for each component before and after node hierarchy generation was confusing. The default used to be ExprResolved<YourType>, and if you had special needs (such as adopting children and including other LayoutNode objects in your post-hierarchy type), you had to manually override that in hierarchy.types.d.ts. This should now just be set in the component config to take effect.
  • Renamed the folder for the Iframe folder to match its component type name (IFrame)
  • Cleaned up and simplified some types. Now HComponent<T> and AnyItem<T> is practically the same (although the first one does not support groups).. 🤷
  • Removing a sneaky disabled property that could be optionally set on all layout components, but was never read by any code.
  • Moving the NotInLayout type. It was now very easy to move these property extensions (such as baseComponentId) to indicate that they are only present on nodes in the hiearchy (as that's what add these properties), not some magic extensions to the layout definitions.
  • Making it possible for a component type to extend the LayoutNode class and construct the extended class in the hierarchy. This will make it possible for components to add functionality here, such as having a LayoutNodeRepGroup type.

Related Issue(s)

Verification/QA

  • Manual functionality testing
    • I have tested these changes manually
    • Creator of the original issue (or service owner) has been contacted for manual testing (or will be contacted when released in alpha)
    • No testing done/necessary
  • Automated tests
    • Unit test(s) have been added/updated
    • Cypress E2E test(s) have been added/updated
    • No automatic tests are needed here (no functional changes/additions)
    • I want someone to help me make some tests
  • UU/WCAG (follow these guidelines until we have our own)
    • I have tested with a screen reader/keyboard navigation/automated wcag validator
    • No testing done/necessary (no DOM/visual changes)
    • I want someone to help me perform accessibility testing
  • User documentation @ altinn-studio-docs
    • Has been added/updated
    • No functionality has been changed/added, so no documentation is needed
    • I will do that later/have created an issue
  • Changes/additions to component properties
    • Changes are reflected in both src/layout/layout.d.ts and layout.schema.v1.json, and these are all backwards-compatible
    • No changes made
  • Support in Altinn Studio
    • Issue(s) created for support in Studio
    • This change/feature does not require any changes to Altinn Studio
  • Sprint board
    • The original issue (or this PR itself) has been added to the Team Apps project and to the current sprint board
    • I don't have permissions to do that, please help me out
  • Labels
    • I have added a kind/* label to this PR for proper release notes grouping
    • I don't have permissions to add labels, please help me out

Ole Martin Handeland and others added 8 commits May 25, 2023 16:25
… the mapped types used everywhere to look up component properties before/after hierarchy-processing
…tomatically generate it when building the app instead. This way a developer can add their own layout components without ever getting any merge conflicts, getting us one step closer to a solution to #200
…tensions. I feared this change would be a lot more difficult, so the fact that this went so smoothly shows that we have come a long way in not relying on the older layout types!
@olemartinorg olemartinorg added the kind/other Pull requests containing chores/repo structure/other changes label May 26, 2023
src/layout/components.gen.mjs Fixed Show fixed Hide fixed
src/layout/components.gen.mjs Fixed Show fixed Hide fixed
Ole Martin Handeland added 3 commits May 30, 2023 10:23
Copy link
Member

@bjosttveit bjosttveit left a comment

Choose a reason for hiding this comment

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

Very cool! I like that you split the config and type config, much cleaner this way!

@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 12 Code Smells

56.7% 56.7% Coverage
0.0% 0.0% Duplication

@olemartinorg olemartinorg merged commit 1767314 into main May 30, 2023
@olemartinorg olemartinorg deleted the chore/refactoring-node branch May 30, 2023 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/other Pull requests containing chores/repo structure/other changes
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants