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

Consolidate templates for Fiori elements FPM #39

Merged
merged 59 commits into from
Nov 4, 2021
Merged

Consolidate templates for Fiori elements FPM #39

merged 59 commits into from
Nov 4, 2021

Conversation

tobiasqueck
Copy link
Contributor

@tobiasqueck tobiasqueck commented Sep 10, 2021

The goal is to provide templates for adding custom extensions to a Fiori elements for OData v4 application that can be used in the SAP Fiori tools (in guided development as well as page modeler) that follow the guidelines provided in the Fiori elements flexible programming model explorer at https://ui5.sap.com/test-resources/sap/fe/core/fpmExplorer/index.html#/customElements/customElementsOverview

The current state is a draft implementation to get a better understanding what is needed for #73

@tobiasqueck
Copy link
Contributor Author

@nlunets I noticed that in the FPM explorer you add custom pages as components to a Fiori elements application, however, when using the page modeler in the SAP Fiori tools, custom pages are added as views.
Both work technically but what would be your recommendation?

@Klaus-Keller FYI

@nlunets
Copy link
Member

nlunets commented Sep 10, 2021

The recommendation would be to add them as components as this enables the usage of building blocks inside the custom page :)

Copy link
Member

@nlunets nlunets left a comment

Choose a reason for hiding this comment

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

Overall it looks good (simple but good).
I'm wondering if we should add more details / options (like how to extend controller extensions and the like in the controller)
Small comments to change

@Klaus-Keller
Copy link
Contributor

@nlunets, what is the minimum UI5 version this template can be used?

@SAP SAP deleted a comment from 815are Sep 20, 2021
@nlunets
Copy link
Member

nlunets commented Sep 20, 2021

I would say 1.93

@815are
Copy link
Contributor

815are commented Sep 22, 2021

I would say 1.93

@nlunets
is it regarding component or controller approach? In tool-suite we have two templates using controller approach:

  1. For versions above 1.88 with sap/fe/core/PageController
  2. For versions below 1.88 with sap/ui/core/mvc/Controller instead of sap/fe/core/PageController

Main explanation was that sap/fe/core/PageController is available since 1.88 - https://sapui5.hana.ondemand.com/#/api/sap.fe.core.PageController

@changeset-bot
Copy link

changeset-bot bot commented Oct 21, 2021

🦋 Changeset detected

Latest commit: daddfd3

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@sap-ux/fe-fpm-writer Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@tobiasqueck tobiasqueck changed the title DRAFT: consolidate templates for Fiori elements FPM Consolidate templates for Fiori elements FPM Nov 3, 2021
stefanschreck
stefanschreck previously approved these changes Nov 3, 2021
Copy link

@stefanschreck stefanschreck left a comment

Choose a reason for hiding this comment

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

OK for me, but a merge from main seems to be required

@tobiasqueck
Copy link
Contributor Author

tobiasqueck commented Nov 4, 2021

Before merging, I would need an approval for each of the following aspects

.vscode/launch.json Outdated Show resolved Hide resolved
nlunets
nlunets previously approved these changes Nov 4, 2021
Copy link
Member

@nlunets nlunets 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 content wise

stefanschreck
stefanschreck previously approved these changes Nov 4, 2021
@devinea devinea dismissed stale reviews from stefanschreck and nlunets via 920b18e November 4, 2021 16:12
nlunets
nlunets previously approved these changes Nov 4, 2021
Copy link
Member

@devinea devinea left a comment

Choose a reason for hiding this comment

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

@tobiasqueck some comments included.

Thanks

packages/fe-fpm-writer/package.json Show resolved Hide resolved
packages/fe-fpm-writer/package.json Show resolved Hide resolved
packages/fe-fpm-writer/src/action/index.ts Show resolved Hide resolved
packages/fe-fpm-writer/src/column/index.ts Show resolved Hide resolved
packages/fe-fpm-writer/tsconfig.json Outdated Show resolved Hide resolved
packages/fe-fpm-writer/src/common/types.ts Show resolved Hide resolved
packages/fe-fpm-writer/src/common/validate.ts Show resolved Hide resolved
Co-authored-by: Austin Devine <devinea@users.noreply.github.com>
Co-authored-by: Austin Devine <devinea@users.noreply.github.com>
Copy link
Member

@devinea devinea left a comment

Choose a reason for hiding this comment

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

Thanks @tobiasqueck lgtm and follow-up TBI created #170

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.

9 participants