feat: display session details in session page Renku 2.0 #3258

andre-code · 2024-07-29T12:40:24Z

PR to address small issues in Renku v2

Issues:

Display Launcher names in session page
When I go to Resources in a Renku 2.0 session, I see the cheat sheet for the Renku CLI (this should be removed)

What is included in this PR:

Show the project name and launcher name in the session view navbar title.
Open a modal with project name and session details when the title in the navbar is clicked.
Remove the Resources Modal, so only session log modal is displayed on the session page.

/deploy

RenkuBot · 2024-07-29T12:41:11Z

You can access the deployment of this PR at https://renku-ci-ui-3258.dev.renku.ch

lorenzo-cavazzi

Nice changes!

The code looks overall good, but there are a few details that require adjustments before merging -- see inline comments.

lorenzo-cavazzi · 2024-08-13T08:42:15Z

client/src/features/sessionsV2/ShowSessionPage.tsx

            )}
          >
-            <div className={cx("px-3", "text-white")}>{sessionName}</div>
+            <div className={cx("px-3", "text-truncate")}>


This is to adjust the vertical alignment -- the difference is subtle

Suggested change

<div className={cx("px-3", "text-truncate")}>

<div className={cx("d-flex", "px-3", "text-truncate")}>

lorenzo-cavazzi · 2024-08-13T08:50:30Z

client/src/features/sessionsV2/ShowSessionPage.tsx

@@ -220,10 +232,17 @@ export default function ShowSessionPage() {
              "d-flex",
              "flex-grow-1",
              "justify-content-between",
-              "py-2"
+              "py-2",
+              "text-truncate"


text-truncate is actually a good idea here 👍

I suggest adjusting this and the two children components to improve the UX. I see the following 2 problems:

With the vertical padding here and no padding in the button for opening the modal, the vertical clickable area is reduced.

Removing the padding here and leaving the button padding would be better. You can adjust the icon position by playing with align-items-center.

(nitpick) There is a bigger gap between the text and the logo when the space is tight

After addressing the previous point, you might need to change the padding in the icon. You probably need only the right padding (pe-3, depending on the other code changes).

lorenzo-cavazzi · 2024-08-13T08:53:42Z

client/src/features/sessionsV2/ShowSessionPage.tsx

+      slug: project?.slug,
+    });
+
+  if (isLoadingLaunchers || isLoadingProject) return <Loader />;


The big bouncing dots here aren't ideal. They also show up in a dark color (see video).

A small white loader would probably be enough. But feel free to change it further if you feel you can make it better (perhaps a short text, something like Checking session details?)

lorenzo-cavazzi · 2024-08-13T08:57:02Z

client/src/features/sessionsV2/ShowSessionPage.tsx

      >
-        <Journals className="bi" />
-        <span className="visually-hidden">Resources</span>
+        <FileEarmarkText className={cx("bi", "me-1")} />


The me-1 class adds an unnecessary black space on the right

Suggested change

<FileEarmarkText className={cx("bi", "me-1")} />

<FileEarmarkText className="bi" />

lorenzo-cavazzi · 2024-08-13T09:06:27Z

client/src/features/sessionsV2/ShowSessionPage.tsx

+    });
+
+  if (isLoadingLaunchers || isLoadingProject) return <Loader />;
+  if (launchersError || !launcher) return <i>Session not accessible</i>;


This can also be improved

I suggest putting it into a text element (like a <p>) styling it to have no bottom margin, white text color, and italics. Optional: add a warning icon at the beginning of the text.

lorenzo-cavazzi · 2024-08-13T09:29:59Z

client/src/features/sessionsV2/ShowSessionPage.tsx

+  const detailsModal = project && session && projectUrl && (
+    <Modal backdrop="static" centered isOpen={isOpen} size="lg" toggle={toggle}>
+      <ModalHeader toggle={toggle}>Session details {launcher.name}</ModalHeader>
+      <ModalBody>


The spacing is a little messed up here: you always add the class mb-2 to the divs, but sometimes they contain a single p child with a bigger bottom margin (equivalent to mb-3) making the class useless, while in other cases the margin is applied but other components add additional vertical margins (see CommandCopy).

I suggest:

Remove the margins within the child components. Depending on the case, you might drop the div entirely leaving a p className="mb-0", or drop the mb-2 on the div, or use properties to remove the margins like in <CommandCopy noMargin ... />.

Handle the spacing between the children at the parent component by adding d-flex, flex-column, gap-3

Show session name in show session page and open session details, remove docs and cheat sheet modal

andre-code · 2024-08-20T08:53:47Z

Thank you, @lorenzo-cavazzi, for your review and suggestions. I've applied those changes in the latest commit. They make a lot of sense!

lorenzo-cavazzi

Looks great! Just a tiny detail before merging

lorenzo-cavazzi · 2024-08-21T10:37:22Z

client/src/features/sessionsV2/ShowSessionPage.tsx

+            className={cx(
+              "d-block",
+              "d-lg-flex",
+              "pb-2",
+              "pb-lg-0",
+              "gap-2",
+              "align-items-center"
+            )}


No need to add additional space on the bottom since the spacing is already handled by the parent component with gap-3

Suggested change

className={cx(

"d-block",

"d-lg-flex",

"pb-2",

"pb-lg-0",

"gap-2",

"align-items-center"

)}

className={cx(

"d-block",

"d-lg-flex",

"gap-2",

"align-items-center"

)}

thanks @lorenzo-cavazzi I have removed the unnecessary style

lorenzo-cavazzi

🚀

RenkuBot · 2024-08-26T12:10:59Z

Tearing down the temporary RenkuLab deplyoment for this PR.

andre-code had a problem deploying to renku-ci-ui-3258 July 29, 2024 12:40 — with GitHub Actions Error

andre-code changed the base branch from main to lorenzo/bootstrap-design July 29, 2024 12:43

andre-code temporarily deployed to renku-ci-ui-3258 July 29, 2024 12:43 — with GitHub Actions Inactive

Base automatically changed from lorenzo/bootstrap-design to main July 30, 2024 09:05

andre-code force-pushed the andrea/issues-renku-2.0 branch 2 times, most recently from 5e10098 to 2f82d5c Compare July 30, 2024 10:27

andre-code had a problem deploying to renku-ci-ui-3258 July 30, 2024 10:27 — with GitHub Actions Error

andre-code force-pushed the andrea/issues-renku-2.0 branch from 2f82d5c to 5289676 Compare July 30, 2024 10:29

andre-code had a problem deploying to renku-ci-ui-3258 July 30, 2024 10:29 — with GitHub Actions Failure

andre-code temporarily deployed to renku-ci-ui-3258 July 30, 2024 11:37 — with GitHub Actions Inactive

andre-code force-pushed the andrea/issues-renku-2.0 branch from 5289676 to 5b863f7 Compare July 30, 2024 12:08

andre-code had a problem deploying to renku-ci-ui-3258 July 30, 2024 12:08 — with GitHub Actions Failure

andre-code temporarily deployed to renku-ci-ui-3258 July 30, 2024 12:48 — with GitHub Actions Inactive

andre-code had a problem deploying to renku-ci-ui-3258 July 30, 2024 13:33 — with GitHub Actions Error

andre-code had a problem deploying to renku-ci-ui-3258 July 30, 2024 13:35 — with GitHub Actions Error

andre-code changed the title ~~[WIP] feat: fix small issues v2~~ feat: display session details in session page Renku 2.0 Jul 30, 2024

andre-code had a problem deploying to renku-ci-ui-3258 July 30, 2024 13:36 — with GitHub Actions Failure

andre-code temporarily deployed to renku-ci-ui-3258 July 30, 2024 14:49 — with GitHub Actions Inactive

andre-code marked this pull request as ready for review July 30, 2024 15:08

andre-code requested a review from a team as a code owner July 30, 2024 15:08

andre-code temporarily deployed to renku-ci-ui-3258 August 12, 2024 08:37 — with GitHub Actions Inactive

lorenzo-cavazzi requested changes Aug 13, 2024

View reviewed changes

lorenzo-cavazzi self-assigned this Aug 13, 2024

feat: fix small issues renku 2.0

8cbe1a2

Show session name in show session page and open session details, remove docs and cheat sheet modal

andre-code force-pushed the andrea/issues-renku-2.0 branch from bfbc368 to 8cbe1a2 Compare August 19, 2024 15:43

andre-code had a problem deploying to renku-ci-ui-3258 August 19, 2024 15:44 — with GitHub Actions Failure

apply code suggestions

8f8b22f

andre-code had a problem deploying to renku-ci-ui-3258 August 20, 2024 08:47 — with GitHub Actions Error

andre-code temporarily deployed to renku-ci-ui-3258 August 20, 2024 08:52 — with GitHub Actions Inactive

andre-code requested a review from lorenzo-cavazzi August 20, 2024 14:35

lorenzo-cavazzi requested changes Aug 21, 2024

View reviewed changes

remove no needed button space

ab1651b

andre-code temporarily deployed to renku-ci-ui-3258 August 23, 2024 08:41 — with GitHub Actions Inactive

andre-code deployed to renku-ci-ui-3258 August 23, 2024 09:13 — with GitHub Actions View deployment

andre-code requested a review from lorenzo-cavazzi August 23, 2024 09:35

lorenzo-cavazzi approved these changes Aug 23, 2024

View reviewed changes

andre-code merged commit fce2c93 into main Aug 26, 2024
18 checks passed

andre-code deleted the andrea/issues-renku-2.0 branch August 26, 2024 12:10

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: display session details in session page Renku 2.0 #3258

feat: display session details in session page Renku 2.0 #3258

andre-code commented Jul 29, 2024 •

edited

Loading

RenkuBot commented Jul 29, 2024

lorenzo-cavazzi left a comment

lorenzo-cavazzi Aug 13, 2024

lorenzo-cavazzi Aug 13, 2024

lorenzo-cavazzi Aug 13, 2024

lorenzo-cavazzi Aug 13, 2024

lorenzo-cavazzi Aug 13, 2024

lorenzo-cavazzi Aug 13, 2024

andre-code commented Aug 20, 2024

lorenzo-cavazzi left a comment •

edited

Loading

lorenzo-cavazzi Aug 21, 2024

andre-code Aug 23, 2024

lorenzo-cavazzi left a comment

RenkuBot commented Aug 26, 2024

	<div className={cx("px-3", "text-truncate")}>
	<div className={cx("d-flex", "px-3", "text-truncate")}>

	<FileEarmarkText className={cx("bi", "me-1")} />
	<FileEarmarkText className="bi" />

feat: display session details in session page Renku 2.0 #3258

feat: display session details in session page Renku 2.0 #3258

Conversation

andre-code commented Jul 29, 2024 • edited Loading

RenkuBot commented Jul 29, 2024

lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

andre-code commented Aug 20, 2024

lorenzo-cavazzi left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lorenzo-cavazzi left a comment

Choose a reason for hiding this comment

RenkuBot commented Aug 26, 2024

andre-code commented Jul 29, 2024 •

edited

Loading

lorenzo-cavazzi left a comment •

edited

Loading