feat(jss): Updated to use JSS and migrate styles from patternfly's react styles by dlabaj · Pull Request #60 · patternfly/react-console

dlabaj · 2024-07-19T05:32:33Z

Updated code to use JSS and remove the need to import the styles from react styles. A follow up PR will be sent out to remove the console styles from patternfly repo. This resolves issue #36 .

Note there is also support for corepack users added to this PR.

Click here to see documentation preview.

nicolethoen

I wonder how many of these CSS overrides we could actually remove now on V6. if not removed, many will need to be updated in this PR or a follow up

nicolethoen · 2024-07-22T19:21:10Z

packages/module/src/components/AccessConsoles/AccessConsoles.tsx

+    display: 'grid',
+    gridTemplateAreas:
+      `'actions-main actions-extra' + 'main main'`,
+      rowGap: 'var(--pf-v6-global--spacer--md)'


These global variables will not exist in v6 - they are replaced with things that start with --pf-t-

nicolethoen · 2024-07-22T19:22:49Z

packages/module/src/components/SerialConsole/XTermStyles.tsx

+      resize: 'none'
+    },
+    '.xterm .composition-view': {
+      background: '#000',


There are tokens that can be used in lieu of hard coded color values

nicolethoen · 2024-07-22T19:25:11Z

packages/module/src/components/VncConsole/VncActions.tsx

+  },
+  consoleActionsVnc: {
+    gridArea: 'actions-extra',
+    display: 'flex',


I wonder if we can use a flex layout here instead of all these flex related css overrides

Makes sense we should add this. Created an issue for this so that we can add this without slowing down the release as this will require some rework (#62).

nicolethoen · 2024-07-22T19:25:16Z

packages/module/src/components/VncConsole/VncConsole.tsx

+  },
+  consoleActionsVnc: {
+    gridArea: 'actions-extra',
+    display: 'flex',


I wonder if we can use a flex layout here instead of all these flex related css overrides

Created #62

wise-king-sullyman

Not sure if it's just related to the above style issues but it does look like something is off with the styling in the example at least, i.e. the console loads in over the controls, and the controls look funky before the console loads.

fhlavac · 2024-07-24T07:13:09Z

packages/module/src/components/AccessConsoles/AccessConsoles.tsx

  });
  return (
-    <div className={css(styles.console)}>
+    <div className={clsx(styles.console)}>


Suggested change

<div className={clsx(styles.console)}>

<div className={styles.console}>

I think there is no need to use clsx in such cases

fhlavac · 2024-07-24T07:14:04Z

packages/module/src/components/SerialConsole/SerialConsoleActions.tsx

+}: SerialConsoleActionsProps) => {
+  const styles = useStyles();
+  return (
+  <div className={clsx(styles.consoleActionsSerial)}>


Suggested change

<div className={clsx(styles.consoleActionsSerial)}>

<div className={styles.consoleActionsSerial}>

fhlavac · 2024-07-24T07:14:19Z

packages/module/src/components/SerialConsole/XTerm.tsx

  // ensure react never reuses this div by keying it with the terminal widget
  // Workaround for xtermjs/xterm.js#3172
-  return <div ref={ref} className="pf-v6-c-console__xterm" role="list" onFocus={onFocusIn} onBlur={onFocusOut} />;
+  return <div ref={ref} className={clsx(styles['.xterm'])} role="list" onFocus={onFocusIn} onBlur={onFocusOut} />;


Suggested change

return <div ref={ref} className={clsx(styles['.xterm'])} role="list" onFocus={onFocusIn} onBlur={onFocusOut} />;

return <div ref={ref} className={styles['.xterm']} role="list" onFocus={onFocusIn} onBlur={onFocusOut} />;

fhlavac · 2024-07-24T07:15:02Z

packages/module/src/components/VncConsole/VncConsole.tsx

    <>
      {rightContent}
-      <div className={css(styles.consoleVnc)}>
+      <div className={clsx(styles.consoleVnc)}>


Suggested change

<div className={clsx(styles.consoleVnc)}>

<div className={styles.consoleVnc}>

fhlavac · 2024-07-24T07:15:15Z

packages/module/src/components/VncConsole/VncActions.tsx

  };
  const toolbar = (
-    <div className={css(styles.consoleActionsVnc)}>
+    <div className={clsx(styles.consoleActionsVnc)}>


Suggested change

<div className={clsx(styles.consoleActionsVnc)}>

<div className={styles.consoleActionsVnc}>

fhlavac · 2024-07-24T07:15:51Z

packages/module/src/components/SerialConsole/SerialConsole.tsx

        />
      )}
-      <div className={css(styles.consoleSerial)}>{terminal}</div>
+      <div className={clsx(styles.consoleSerial)}>{terminal}</div>


Suggested change

<div className={clsx(styles.consoleSerial)}>{terminal}</div>

<div className={styles.consoleSerial}>{terminal}</div>

fhlavac · 2024-07-24T07:16:12Z

packages/module/src/components/DesktopViewer/DesktopViewer.tsx

+}: DesktopViewerProps) => {
+  const styles = useStyles();
+  return (
+  <div className={clsx(styles.consoleDesktopViewer)}>


Suggested change

<div className={clsx(styles.consoleDesktopViewer)}>

<div className={styles.consoleDesktopViewer}>

fhlavac · 2024-07-24T07:16:32Z

packages/module/src/components/AccessConsoles/AccessConsoles.tsx

+    <div className={clsx(styles.console)}>
      {React.Children.toArray(children).length > 1 && (
-        <div className={css(styles.consoleActions)}>
+        <div className={clsx(styles.consoleActions)}>


Suggested change

<div className={clsx(styles.consoleActions)}>

<div className={styles.consoleActions}>

…jss. Also added corepack support for legacy yarn.

dlabaj · 2024-07-24T22:59:45Z

Not sure if it's just related to the above style issues but it does look like something is off with the styling in the example at least, i.e. the console loads in over the controls, and the controls look funky before the console loads.

Fixed and updated the code.

nicolethoen · 2024-07-25T19:55:39Z

packages/module/src/css/xterm.css

+
+.xterm .composition-view {
+    /* TODO: Composition position got messed up somewhere */
+    background: #000;


Can colors in this file be updated to tokens rather than hardcoded hex values?

Got it. Thanks nicole.

github-actions · 2024-07-25T21:36:50Z

🎉 This PR is included in version 6.0.0-alpha.4 🎉

The release is available on:

Your semantic-release bot 📦🚀

github-actions · 2024-09-18T18:58:56Z

🎉 This PR is included in version 6.0.0-prerelease.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

dlabaj requested review from nicolethoen and wise-king-sullyman July 19, 2024 05:32

dlabaj mentioned this pull request Jul 19, 2024

Remove console styles patternfly/patternfly-react#10780

Merged

dlabaj requested a review from kmcfaul July 19, 2024 06:00

dlabaj linked an issue Jul 22, 2024 that may be closed by this pull request

Move CSS from react-styles into react-console #36

Closed

nicolethoen reviewed Jul 22, 2024

View reviewed changes

wise-king-sullyman reviewed Jul 22, 2024

View reviewed changes

fhlavac reviewed Jul 24, 2024

View reviewed changes

feat(styles): Moved console react-styles to react-console, and added …

643a9ce

…jss. Also added corepack support for legacy yarn.

dlabaj force-pushed the console-styles branch from ee59ea9 to 643a9ce Compare July 24, 2024 22:58

nicolethoen reviewed Jul 25, 2024

View reviewed changes

wise-king-sullyman approved these changes Jul 25, 2024

View reviewed changes

Updated with review comments.

f5c641f

dlabaj merged commit 993717c into patternfly:v6 Jul 25, 2024

github-actions bot added the released on @alpha label Jul 25, 2024

github-actions bot added the released on @prerelease label Sep 18, 2024

	<div className={clsx(styles.console)}>
	<div className={styles.console}>

	<div className={clsx(styles.consoleActionsSerial)}>
	<div className={styles.consoleActionsSerial}>

	return <div ref={ref} className={clsx(styles['.xterm'])} role="list" onFocus={onFocusIn} onBlur={onFocusOut} />;
	return <div ref={ref} className={styles['.xterm']} role="list" onFocus={onFocusIn} onBlur={onFocusOut} />;

	<div className={clsx(styles.consoleVnc)}>
	<div className={styles.consoleVnc}>

	<div className={clsx(styles.consoleActionsVnc)}>
	<div className={styles.consoleActionsVnc}>

	<div className={clsx(styles.consoleSerial)}>{terminal}</div>
	<div className={styles.consoleSerial}>{terminal}</div>

	<div className={clsx(styles.consoleDesktopViewer)}>
	<div className={styles.consoleDesktopViewer}>

	<div className={clsx(styles.consoleActions)}>
	<div className={styles.consoleActions}>

Conversation

dlabaj commented Jul 19, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolethoen left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wise-king-sullyman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dlabaj commented Jul 24, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jul 25, 2024

Uh oh!

github-actions bot commented Sep 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

dlabaj commented Jul 19, 2024 •

edited

Loading