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

Chore: make a bunch of components standalone #1047

Merged
merged 14 commits into from
Nov 26, 2024
Merged

Conversation

jahow
Copy link
Collaborator

@jahow jahow commented Nov 20, 2024

Description

The impacted components are:

  • organization-details.component.ts
  • organization-header.component.ts
  • header-record.component.ts
  • record-apis.component.ts
  • record-downloads.component.ts
  • record-metadata.component.ts
  • record-otherlinks.component.ts
  • record-page.component.ts
  • record-related-records.component.ts
  • record-user-feedbacks.component.ts
  • records-list.component.ts
  • organisations.component.ts
  • chart-view.component.ts
  • figure-container.component.ts
  • geo-table-view.component.ts
  • table-view.component.ts
  • layers-panel.component.ts
  • data-view-permalink.component.ts
  • data-view-share.component.ts
  • view-web-component.component.ts
  • data-view.component.ts
  • external-viewer-button.component.ts
  • map-view.component.ts
  • record-meta.component.ts
  • favorite-star.component.ts
  • catalog-title.component.ts
  • language-switcher.component.ts
  • organisation-preview.component.ts
  • organisations-result.component.ts
  • card/api-card.component.ts
  • ghost/content-ghost.component.ts
  • download-item.component.ts
  • downloads-list.component.ts
  • error.component.ts
  • image-overlay-preview.component.ts
  • metadata-catalog.component.ts
  • metadata-contact.component.ts
  • metadata-info.component.ts
  • metadata-quality-item.component.ts
  • metadata-quality.component.ts
  • pagination-buttons.component.ts
  • pagination.component.ts
  • record-api-form.component.ts
  • related-record-card.component.ts
  • user-feedback-item.component.ts
  • copy-text-button.component.ts
  • navigation-button.component.ts
  • star-toggle.component.ts
  • expandable-panel-button.component.ts
  • expandable-panel.component.ts
  • loading-mask.component.ts
  • progress-bar.component.ts
  • spinning-loader.component.ts

Quality Assurance Checklist

  • Commit history is devoid of any merge commits and readable to facilitate reviews
  • If new logic ⚙️ is introduced: unit tests were added
  • If new user stories 🤏 are introduced: E2E tests were added
  • If new UI components 🕹️ are introduced: corresponding stories in Storybook were created
  • If breaking changes 🪚 are introduced: add the breaking change label
  • If bugs 🐞 are fixed: add the backport <release branch> label
  • The documentation website 📚 has received the love it deserves

@jahow
Copy link
Collaborator Author

jahow commented Nov 20, 2024

Note: I reverted #1042 to make the tests pass but this will have to be done separately (if we really do it)

Copy link
Contributor

github-actions bot commented Nov 20, 2024

Affected libs: feature-catalog, feature-record, feature-router, feature-dataviz, feature-map, feature-search, ui-catalog, ui-dataviz, ui-search, ui-elements, feature-notifications, feature-editor, ui-inputs, ui-layout, ui-widgets,
Affected apps: datafeeder, datahub, metadata-editor, search, webcomponents, demo, map-viewer, metadata-converter,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

Copy link
Contributor

github-actions bot commented Nov 20, 2024

📷 Screenshots are here!

@jahow jahow force-pushed the make-standalone-components branch 3 times, most recently from 419cd22 to b1557ec Compare November 21, 2024 14:50
@coveralls
Copy link

coveralls commented Nov 21, 2024

Coverage Status

coverage: 81.467% (-2.6%) from 84.054%
when pulling ce72548 on make-standalone-components
into e8d5d12 on main.

@jahow jahow force-pushed the make-standalone-components branch from b1557ec to 79d4da8 Compare November 21, 2024 22:38
@jahow jahow force-pushed the make-standalone-components branch from 79d4da8 to 210451d Compare November 22, 2024 10:59
Copy link
Collaborator

@tkohr tkohr left a comment

Choose a reason for hiding this comment

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

Pfiou, tedious PR, thanks @jahow ! Took a look over the code, started the datahub, editor and storybook and could only make out some issues on the latter.

Some components like TableView, ApiCardComponent, GnUiLinkifyDirective throw an error like Unexpected "LoadingMaskComponent" found in the "declarations" array of the "StorybookComponentModule" NgModule, "LoadingMaskComponent" is marked as standalone and can't be declared in any NgModule - did you intend to import it instead (by adding it to the "imports" array)? in storybook.

FigureContainerComponent and FigureComponent have issues with the TranslateService, but not sure if that is related to this PR.

LoadingMaskComponent,
SpinningLoaderComponent,
],
declarations: [ColorScaleComponent, StepBarComponent],
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just for better understanding. How did you decide, which components to make standalone in each module and which ones to keep as is for now? Did you encounter any architectural limitations?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question, my approach was to pull on the thread and see what comes. So I'd start with a component that's not standalone and make it standalone and then see what components are affected and make them standalone in turn, etc. :)

@jahow
Copy link
Collaborator Author

jahow commented Nov 26, 2024

thank you @tkohr , indeed many storybook entries were failing! I went through all of them and fixed them.

Copy link
Collaborator

@tkohr tkohr left a comment

Choose a reason for hiding this comment

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

Thanks, working fine now 🤩, just the e2e are red.

@jahow jahow merged commit 3126400 into main Nov 26, 2024
13 checks passed
@jahow jahow deleted the make-standalone-components branch November 26, 2024 18:06
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.

3 participants