You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@kie.apache.org by "handreyrc (via GitHub)" <gi...@apache.org> on 2024/04/15 21:23:54 UTC
[PR] kie-issues#2215: Combined editor > The loading screen is kept in light mode [incubator-kie-tools]
handreyrc opened a new pull request, #2242:
URL: https://github.com/apache/incubator-kie-tools/pull/2242
Closes: [incubator-kie-tools/issues#2215](https://github.com/apache/incubator-kie-tools/issues/2215)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-tools-issues#2215: Combined editor > The loading screen is kept in light mode [incubator-kie-tools]
Posted by "handreyrc (via GitHub)" <gi...@apache.org>.
handreyrc commented on PR #2242:
URL: https://github.com/apache/incubator-kie-tools/pull/2242#issuecomment-2078306135
Hey @caponetto,
Please, could you check this PR?
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-tools-issues#2215: Combined editor > The loading screen is kept in light mode [incubator-kie-tools]
Posted by "handreyrc (via GitHub)" <gi...@apache.org>.
handreyrc commented on code in PR #2242:
URL: https://github.com/apache/incubator-kie-tools/pull/2242#discussion_r1581434937
##########
packages/editor/src/envelope/LoadingScreen/LoadingScreen.tsx:
##########
@@ -24,7 +24,7 @@ import { Title } from "@patternfly/react-core/dist/js/components/Title";
import { Bullseye } from "@patternfly/react-core/dist/js/layouts/Bullseye";
import { useEditorEnvelopeI18nContext } from "../i18n";
-export function LoadingScreen(props: { loading: boolean }) {
+export function LoadingScreen(props: { loading: boolean; style?: string }) {
Review Comment:
@caponetto,
Oh I see, actualy I commited the next change below, however, it uses "style" instead of "styleTag" and I ended up fixing the name of the prop insted of it's usages.
It's fixed now!
Please, could you check it again?
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-tools-issues#2215: Combined editor > The loading screen is kept in light mode [incubator-kie-tools]
Posted by "paulovmr (via GitHub)" <gi...@apache.org>.
paulovmr merged PR #2242:
URL: https://github.com/apache/incubator-kie-tools/pull/2242
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-tools-issues#2215: Combined editor > The loading screen is kept in light mode [incubator-kie-tools]
Posted by "caponetto (via GitHub)" <gi...@apache.org>.
caponetto commented on code in PR #2242:
URL: https://github.com/apache/incubator-kie-tools/pull/2242#discussion_r1580836030
##########
packages/editor/src/envelope/LoadingScreen/LoadingScreen.tsx:
##########
@@ -47,9 +47,11 @@ export function LoadingScreen(props: { loading: boolean }) {
}
}, [props.loading]);
+ const style = !props.style ? "kie-tools--loading-screen" : props.style + " kie-tools--loading-screen";
Review Comment:
```suggestion
const style = (props.style ? `${props.style} ` : "") + "kie-tools--loading-screen";
```
Not sure if this is better readable, but it'd be nice to remove the duplication of the hardcoded string `"kie-tools--loading-screen"`.
##########
packages/serverless-workflow-combined-editor/src/editor/ServerlessWorkflowCombinedEditor.tsx:
##########
@@ -486,13 +488,24 @@ const RefForwardingServerlessWorkflowCombinedEditor: ForwardRefRenderFunction<
);
};
-function getThemeStyle(theme: EditorTheme) {
+interface ThemeStyle {
Review Comment:
```suggestion
interface ThemeStyleTag {
```
##########
packages/serverless-workflow-combined-editor/src/editor/ServerlessWorkflowCombinedEditor.tsx:
##########
@@ -462,15 +462,17 @@ const RefForwardingServerlessWorkflowCombinedEditor: ForwardRefRenderFunction<
}
}, [editorEnvelopeCtx, isCombinedEditorReady]);
+ const themeStyle = getThemeStyle(theme!);
+
return (
<div style={{ height: "100%" }}>
Review Comment:
When we are in dark mode, I see a quick white blink during the transition between the loading screen and the editor screen. As far as I can see, this is because we don't set any background color to this div. Do you think you can incorporate a fix for this case into this PR? I believe it is just a matter of setting the `backgroundColor` to a value depending on the theme. In our case, it could be black for dark and white for light. WDYT?
##########
packages/editor/src/envelope/LoadingScreen/LoadingScreen.tsx:
##########
@@ -24,7 +24,7 @@ import { Title } from "@patternfly/react-core/dist/js/components/Title";
import { Bullseye } from "@patternfly/react-core/dist/js/layouts/Bullseye";
import { useEditorEnvelopeI18nContext } from "../i18n";
-export function LoadingScreen(props: { loading: boolean }) {
+export function LoadingScreen(props: { loading: boolean; style?: string }) {
Review Comment:
```suggestion
export function LoadingScreen(props: { loading: boolean; styleTag?: string }) {
```
Just to make it clear that the parameter corresponds to a tag.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-tools-issues#2215: Combined editor > The loading screen is kept in light mode [incubator-kie-tools]
Posted by "handreyrc (via GitHub)" <gi...@apache.org>.
handreyrc commented on code in PR #2242:
URL: https://github.com/apache/incubator-kie-tools/pull/2242#discussion_r1581191567
##########
packages/serverless-workflow-combined-editor/src/editor/ServerlessWorkflowCombinedEditor.tsx:
##########
@@ -462,15 +462,17 @@ const RefForwardingServerlessWorkflowCombinedEditor: ForwardRefRenderFunction<
}
}, [editorEnvelopeCtx, isCombinedEditorReady]);
+ const themeStyle = getThemeStyle(theme!);
+
return (
<div style={{ height: "100%" }}>
Review Comment:
Hey @caponetto ,
Sure, it's done!
Please, could you check it again?
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-tools-issues#2215: Combined editor > The loading screen is kept in light mode [incubator-kie-tools]
Posted by "caponetto (via GitHub)" <gi...@apache.org>.
caponetto commented on code in PR #2242:
URL: https://github.com/apache/incubator-kie-tools/pull/2242#discussion_r1581289239
##########
packages/editor/src/envelope/LoadingScreen/LoadingScreen.tsx:
##########
@@ -24,7 +24,7 @@ import { Title } from "@patternfly/react-core/dist/js/components/Title";
import { Bullseye } from "@patternfly/react-core/dist/js/layouts/Bullseye";
import { useEditorEnvelopeI18nContext } from "../i18n";
-export function LoadingScreen(props: { loading: boolean }) {
+export function LoadingScreen(props: { loading: boolean; style?: string }) {
Review Comment:
@handreyrc I think the commit for this change got overridden on the last force push.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-tools-issues#2215: Combined editor > The loading screen is kept in light mode [incubator-kie-tools]
Posted by "handreyrc (via GitHub)" <gi...@apache.org>.
handreyrc commented on code in PR #2242:
URL: https://github.com/apache/incubator-kie-tools/pull/2242#discussion_r1581434937
##########
packages/editor/src/envelope/LoadingScreen/LoadingScreen.tsx:
##########
@@ -24,7 +24,7 @@ import { Title } from "@patternfly/react-core/dist/js/components/Title";
import { Bullseye } from "@patternfly/react-core/dist/js/layouts/Bullseye";
import { useEditorEnvelopeI18nContext } from "../i18n";
-export function LoadingScreen(props: { loading: boolean }) {
+export function LoadingScreen(props: { loading: boolean; style?: string }) {
Review Comment:
@caponetto,
Oh I see, actually I commited the next change below, however, it uses "style" instead of "styleTag" and I ended up fixing the name of the prop insted of its usages.
It's fixed now!
Please, could you check it again?
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org
Re: [PR] kie-tools-issues#2215: Combined editor > The loading screen is kept in light mode [incubator-kie-tools]
Posted by "handreyrc (via GitHub)" <gi...@apache.org>.
handreyrc commented on code in PR #2242:
URL: https://github.com/apache/incubator-kie-tools/pull/2242#discussion_r1581434937
##########
packages/editor/src/envelope/LoadingScreen/LoadingScreen.tsx:
##########
@@ -24,7 +24,7 @@ import { Title } from "@patternfly/react-core/dist/js/components/Title";
import { Bullseye } from "@patternfly/react-core/dist/js/layouts/Bullseye";
import { useEditorEnvelopeI18nContext } from "../i18n";
-export function LoadingScreen(props: { loading: boolean }) {
+export function LoadingScreen(props: { loading: boolean; style?: string }) {
Review Comment:
@caponetto,
Oh I see, actually I commited the next change below, however, it uses "style" instead of "styleTag" and I ended up fixing the name of the prop insted of it's usages.
It's fixed now!
Please, could you check it again?
Thanks
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@kie.apache.org
For additional commands, e-mail: commits-help@kie.apache.org