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

Update project cards style and layout #5098

Merged
merged 15 commits into from
Aug 27, 2021
Merged

Conversation

gtsiolis
Copy link
Contributor

@gtsiolis gtsiolis commented Aug 6, 2021

A minor facelift on the project cards. 🔮

This will fix #4420 (see #4420 (comment)) and #5003.

BEFORE AFTER
Screenshot 2021-08-25 at 8 25 37 PM Screenshot 2021-08-25 at 8 25 50 PM
Screenshot 2021-08-25 at 8 26 33 PM Screenshot 2021-08-25 at 8 26 36 PM

Copy link
Contributor Author

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Phew! Replaced some classes to use fixed dimensions. Left some links blank to be replaced with links to actual pages. Feel free to pick this up and add the missing links. Cc @AlexTugarev @jankeromnes 🏀

components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
@jankeromnes
Copy link
Contributor

jankeromnes commented Aug 25, 2021

Notes:

  • I've updated this PR to latest gitpod-io/main, and fixed the conflict
  • This created a "Merge Commit", but it will go away automatically when we "Rebase Merge" eventually (EDIT: Or, you can git rebase origin/main and this should also clean up the Merge Commit)
  • Here is exactly the changes I did to this PR to resolve the conflict

Copy link
Contributor

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Looks good to me! Many thanks. 🙏

I've also left some one-click suggestions in-line, e.g. to implement the missing links you pointed out in the comments.

/hold

components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
components/dashboard/src/projects/Projects.tsx Outdated Show resolved Hide resolved
@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 3962bdac4c4a782ac2fcf8331b17458cc012b5ef

@roboquat roboquat removed the lgtm label Aug 25, 2021
@roboquat roboquat removed the lgtm label Aug 25, 2021
@roboquat
Copy link
Contributor

New changes are detected. LGTM label has been removed.

<img className="h-3 w-3 my-auto" src={getPrebuildStatusIcon(lastPrebuilds.get(p.id)!.status)} />
<div className="my-auto font-semibold text-gray-400 truncate w-24" title={lastPrebuilds.get(p.id)!.branch}>{lastPrebuilds.get(p.id)!.branch}</div>
<span className="mx-1 my-auto text-gray-600">·</span>
<div className="my-auto text-gray-500 flex-grow hover:text-gray-800 dark:hover:text-gray-300">{moment(lastPrebuilds.get(p.id)!.startedAt, "YYYYMMDD").fromNow()}</div>
Copy link
Contributor Author

@gtsiolis gtsiolis Aug 25, 2021

Choose a reason for hiding this comment

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

issue: Just noticed that this always shows 20 hours ago for two random projects while trigger time within the prebuild page is different for both. Expected? 💭

@gtsiolis
Copy link
Contributor Author

Posting some before and after screenshots here:

BEFORE AFTER
Screenshot 2021-08-25 at 8 25 37 PM Screenshot 2021-08-25 at 8 25 50 PM
Screenshot 2021-08-25 at 8 26 33 PM Screenshot 2021-08-25 at 8 26 36 PM

@gtsiolis gtsiolis force-pushed the gt/update-project-cards-layout branch from 36b0d55 to 25a4880 Compare August 25, 2021 17:27
@jankeromnes
Copy link
Contributor

jankeromnes commented Aug 26, 2021

(Side note: When pushing minor new changes, please feel free to "carry over" the lgtm label that @roboquat automatically removes -- I don't actually need to re-review entirely if e.g. you fix a typo or so 😁 simply re-adding the lgtm label manually works great here.)

@AlexTugarev
Copy link
Member

/lgtm this is very nice!

@gtsiolis
Copy link
Contributor Author

gtsiolis commented Aug 26, 2021

When pushing minor new changes, ...

Documentation or didn't see this! Cc @leodido 🤓

@gtsiolis gtsiolis added the lgtm label Aug 27, 2021
@gtsiolis
Copy link
Contributor Author

/lgtm

5l0a2z

@roboquat
Copy link
Contributor

@gtsiolis: you cannot LGTM your own PR.

In response to this:

/lgtm

5l0a2z

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: AlexTugarev, gtsiolis, jankeromnes

Associated issue: #4420

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@gtsiolis
Copy link
Contributor Author

/unhold

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link to all and latest prebuild from the project card [Dashboard] Team Projects Page
4 participants