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

Simplify pages/menus/registry extension api internal implementation #1364

Merged
merged 13 commits into from
Nov 13, 2020

Conversation

jakolehm
Copy link
Contributor

@jakolehm jakolehm commented Nov 13, 2020

Simplifies #1334

Includes fix from #1361.

Changes how relation between page and menu is done. Now each page is required to have id, menu can target a page by providing a target object. Target can also be a page from another extension.

ixrock and others added 3 commits November 12, 2020 22:53
Signed-off-by: Roman <ixrock@gmail.com>
Signed-off-by: Roman <ixrock@gmail.com>
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
@jakolehm jakolehm added the area/extension Something to related to the extension api label Nov 13, 2020
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
@jakolehm jakolehm marked this pull request as ready for review November 13, 2020 08:15
@jakolehm jakolehm added the bug Something isn't working label Nov 13, 2020
@jakolehm jakolehm added this to the 4.0.0 milestone Nov 13, 2020
@jakolehm jakolehm requested review from ixrock and a team November 13, 2020 08:16
@nevalla
Copy link
Contributor

nevalla commented Nov 13, 2020

👍 It feels better now when not every extension have pageRoute and pageUrl.

@jakolehm
Copy link
Contributor Author

@ixrock PTAL (I can port remaining improvements from #1361, or rebase if we merge it)

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Copy link
Contributor

@ixrock ixrock left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -20,12 +24,26 @@ export interface PageComponents {
Page: React.ComponentType<any>;
}

const routePrefix = "/extension/:name"

export function getPageUrl(ext: LensExtension, baseUrl = "") {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export function getPageUrl(ext: LensExtension, baseUrl = "") {
export function getPageRoute(ext: LensExtension, routePathOrUrl = "") {

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Copy link
Contributor

@nevalla nevalla left a comment

Choose a reason for hiding this comment

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

LGTM

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
@jakolehm jakolehm merged commit df0f080 into master Nov 13, 2020
@jakolehm jakolehm deleted the simplify-pages-registry-implementation branch November 13, 2020 15:04
@jakolehm jakolehm mentioned this pull request Nov 13, 2020
pauljwil pushed a commit to pauljwil/lens that referenced this pull request Nov 13, 2020
Resolved conflicts while rebasing after pulling from remote

Exit pipeline if extensions build or tests fail (lensapp#1370)

Signed-off-by: Lauri Nevala <lauri.nevala@gmail.com>

Simplify pages/menus/registry extension api internal implementation (lensapp#1364)

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Co-authored-by: Roman <ixrock@gmail.com>

Resolved conflicts in rebase from master

Updated docs to sentence case throughout

Changed mkdoc.yml nav to sentence case

Signed-off-by: Paul Williams <pawilliams@mirantis.com>

Changed headings to sentence case throughtout docs

Signed-off-by: Paul Williams <pawilliams@mirantis.com>

Changed all titles and headings back to title case throughout docs and in mkdocs.yml nav

Signed-off-by: Paul Williams <pawilliams@mirantis.com>

Updated docs to sentence case throughout

Changed mkdoc.yml nav to sentence case

Signed-off-by: Paul Williams <pawilliams@mirantis.com>

Changed headings to sentence case throughtout docs

Signed-off-by: Paul Williams <pawilliams@mirantis.com>

Changed all titles and headings back to title case throughout docs and in mkdocs.yml nav

Signed-off-by: Paul Williams <pawilliams@mirantis.com>
pauljwil pushed a commit to pauljwil/lens that referenced this pull request Nov 13, 2020
Resolved conflicts while rebasing after pulling from remote

Exit pipeline if extensions build or tests fail (lensapp#1370)

Signed-off-by: Lauri Nevala <lauri.nevala@gmail.com>

Simplify pages/menus/registry extension api internal implementation (lensapp#1364)

Signed-off-by: Jari Kolehmainen <jari.kolehmainen@gmail.com>
Co-authored-by: Roman <ixrock@gmail.com>

Resolved conflicts in rebase from master

Updated docs to sentence case throughout

Changed mkdoc.yml nav to sentence case

Signed-off-by: Paul Williams <pawilliams@mirantis.com>

Changed headings to sentence case throughtout docs

Signed-off-by: Paul Williams <pawilliams@mirantis.com>

Changed all titles and headings back to title case throughout docs and in mkdocs.yml nav

Signed-off-by: Paul Williams <pawilliams@mirantis.com>

Updated docs to sentence case throughout

Changed mkdoc.yml nav to sentence case

Signed-off-by: Paul Williams <pawilliams@mirantis.com>

Changed headings to sentence case throughtout docs

Signed-off-by: Paul Williams <pawilliams@mirantis.com>

Changed all titles and headings back to title case throughout docs and in mkdocs.yml nav

Signed-off-by: Paul Williams <pawilliams@mirantis.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/extension Something to related to the extension api bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants