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

kie-issues#130: chrome-extension: Kogito icon is not working correctly after using the browser "back" #2134

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -24,18 +24,25 @@ import {
createAndGetMainContainer,
openRepoInExternalEditorContainer,
openRepoInExternalEditorContainerFromRepositoryHome,
removeAllChildren,
} from "../../utils";
import { OpenInExternalEditorButton } from "./OpenInExternalEditorButton";
import { GitHubPageType } from "../../github/GitHubPageType";
import { KOGITO_OPEN_REPO_IN_EXTERNAL_EDITOR_CONTAINER_CLASS } from "../../constants";
import { cleanupDuplicateElements, waitForElementToBeReady } from "../../utils";

export function renderOpenRepoInExternalEditorApp(
export async function renderOpenRepoInExternalEditorApp(
args: Globals & { className: string; pageType: GitHubPageType; container: () => HTMLElement }
) {
// Necessary because GitHub apparently "caches" DOM structures between changes on History.
// Without this method you can observe duplicated elements when using back/forward browser buttons.
cleanup(args.id);
// wait for the dom element to be ready before rendering.
await waitForElementToBeReady("#repository-details-container");
// Checking whether this text editor exists is a good way to determine if the page is "ready",
// because that would mean that the user could see the default GitHub page.
if (!args.dependencies.openRepoInExternalEditor.buttonContainerOnRepoHome()) {
args.logger.log(`Doesn't look like the GitHub page is ready yet.`);
return;
}

cleanupDuplicateElements(args.id, [KOGITO_OPEN_REPO_IN_EXTERNAL_EDITOR_CONTAINER_CLASS]);

ReactDOM.render(
<Main
Expand All @@ -59,12 +66,3 @@ export function renderOpenRepoInExternalEditorApp(
() => args.logger.log("Mounted.")
);
}

function cleanup(id: string) {
Array.from(document.querySelectorAll(`.${KOGITO_OPEN_REPO_IN_EXTERNAL_EDITOR_CONTAINER_CLASS}.${id}`)).forEach(
(e) => {
removeAllChildren(e);
e.remove();
}
);
}
35 changes: 9 additions & 26 deletions packages/chrome-extension/src/app/components/pr/prEditors.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
import * as ReactDOM from "react-dom";
import * as React from "react";
import { PrEditorsApp } from "./PrEditorsApp";
import { createAndGetMainContainer, openRepoInExternalEditorContainer, removeAllChildren } from "../../utils";
import { createAndGetMainContainer, openRepoInExternalEditorContainer } from "../../utils";
import { Globals, Main } from "../common/Main";
import {
KOGITO_IFRAME_CONTAINER_PR_CLASS,
Expand All @@ -32,6 +32,7 @@ import { Dependencies } from "../../Dependencies";
import { PrInfo } from "./IsolatedPrEditor";
import { OpenInExternalEditorButton } from "../openRepoInExternalEditor/OpenInExternalEditorButton";
import { GitHubPageType } from "../../github/GitHubPageType";
import { cleanupDuplicateElements } from "../../utils";

export function renderPrEditorsApp(
args: Globals & {
Expand All @@ -40,10 +41,13 @@ export function renderPrEditorsApp(
container: () => HTMLElement;
}
) {
// Necessary because GitHub apparently "caches" DOM structures between changes on History.
// Without this method you can observe duplicated elements when using back/forward browser buttons.
cleanup(args.id);

const prEditorSelectorsArray = [
KOGITO_IFRAME_CONTAINER_PR_CLASS,
KOGITO_VIEW_ORIGINAL_LINK_CONTAINER_PR_CLASS,
KOGITO_TOOLBAR_CONTAINER_PR_CLASS,
KOGITO_OPEN_REPO_IN_EXTERNAL_EDITOR_CONTAINER_CLASS,
];
cleanupDuplicateElements(args.id, prEditorSelectorsArray);
ReactDOM.render(
<Main
id={args.id}
Expand Down Expand Up @@ -91,24 +95,3 @@ export function parsePrInfo(dependencies: Dependencies): PrInfo {
gitRef: prInfos[5],
};
}

function cleanup(id: string) {
Array.from(document.querySelectorAll(`.${KOGITO_IFRAME_CONTAINER_PR_CLASS}.${id}`)).forEach((e) => {
removeAllChildren(e);
});

Array.from(document.querySelectorAll(`.${KOGITO_VIEW_ORIGINAL_LINK_CONTAINER_PR_CLASS}.${id}`)).forEach((e) => {
removeAllChildren(e);
});

Array.from(document.querySelectorAll(`.${KOGITO_TOOLBAR_CONTAINER_PR_CLASS}.${id}`)).forEach((e) => {
removeAllChildren(e);
});

Array.from(document.querySelectorAll(`.${KOGITO_OPEN_REPO_IN_EXTERNAL_EDITOR_CONTAINER_CLASS}.${id}`)).forEach(
(e) => {
removeAllChildren(e);
e.remove();
}
);
}
11 changes: 11 additions & 0 deletions packages/chrome-extension/src/app/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,7 @@ export function iframeFullscreenContainer(id: string, container: HTMLElement) {
}

export function kogitoMenuContainer(id: string, container: HTMLElement) {
cleanupDuplicateElements(id, [KOGITO_MENU_CONTAINER_CLASS]);
const element = () => document.querySelector(`.${KOGITO_MENU_CONTAINER_CLASS}.${id}`)!;

if (!element()) {
Expand Down Expand Up @@ -161,3 +162,13 @@ export function waitForElementToBeReady(selector: string) {
});
});
}

// Necessary because GitHub apparently "caches" DOM structures between changes on History.
// Without this method you can observe duplicated elements when using back/forward browser buttons.
export function cleanupDuplicateElements(id: string, selectors: string[]) {
selectors.forEach((selector) => {
Array.from(document.querySelectorAll(`.${selector}.${id}`)).forEach((e) => {
removeAllChildren(e);
});
});
}
Comment on lines +168 to +174
Copy link
Contributor

Choose a reason for hiding this comment

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

Currently this function is receiving classes and building a selector in a strictly way. You could improve this function by building the selectors before passing it down to the function. This way the caller can pass down a different selector if required.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point thanks Luiz, will update it accordingly

Loading