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

[projects] Fix Project card bottom row layout #6187

Merged
merged 1 commit into from
Oct 14, 2021
Merged

Conversation

jankeromnes
Copy link
Contributor

Description

Fixes the bottom row flex layout of Project cards.

Related Issue(s)

Fixes #5978

How to test

See #5978 (comment)

Release Notes

[projects] Fix Project card bottom row layout

Documentation

/uncc

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Oct 13, 2021

/werft run

👍 started the job as gitpod-build-jx-fix-project-card.1

@jankeromnes jankeromnes marked this pull request as ready for review October 13, 2021 10:29
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Oct 13, 2021

Ready for review! 😁 This is what it now looks like:

"Normal" screen (e.g. 15" retina) Larger screen (e.g. external monitor)
Screenshot 2021-10-13 at 12 28 47 Screenshot 2021-10-13 at 12 28 59

@gtsiolis could you please take a look when you have some time? 🙏

@gtsiolis
Copy link
Contributor

gtsiolis commented Oct 13, 2021

Looking at this now! 👀

Copy link
Contributor

@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.

Flawless, as usual, @jankeromnes! 🌟

Left one minor suggestion below.

Approving to unblock merging but holding in case we could squeeze also that small spacing issue. 🍊

/hold

@@ -151,16 +151,14 @@ export default function () {
</div>
<div className="h-10 px-4 border rounded-b-xl dark:border-gray-800 bg-gray-100 border-gray-100 dark:bg-gray-800">
{lastPrebuilds.get(p.id)
? (<div className="flex flex-row h-full text-sm justify-between">
<Link to={`/${teamOrUserSlug}/${p.name}/${lastPrebuilds.get(p.id)?.info?.id}`} className="flex my-auto items-center group space-x-2">
? (<div className="flex flex-row h-full text-sm space-x-2">
Copy link
Contributor

@gtsiolis gtsiolis Oct 13, 2021

Choose a reason for hiding this comment

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

suggestion: What do you think of adding some more space between the latest prebuild and the link to view all prebuilds? See also principles of grouping. 🤓

Suggested change
? (<div className="flex flex-row h-full text-sm space-x-2">
? (<div className="flex flex-row h-full text-sm space-x-4">
BEFORE AFTER
spacing-before spacing-after

@roboquat
Copy link
Contributor

LGTM label has been added.

Git tree hash: 791410649cbe5784d2372b8f832d06b39f8bca7a

@roboquat
Copy link
Contributor

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: gtsiolis

Associated issue: #5978

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

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Oct 14, 2021

Flawless, as usual, @jankeromnes! 🌟

So much praise 😊 thank you for the flawless review, as usual!

suggestion: What do you think of adding some more space between the latest prebuild and the link to view all prebuilds? See also principles of grouping. 🤓

I like it! Thank you for the small touch that makes the entire thing much nicer. ✨

@roboquat
Copy link
Contributor

New changes are detected. LGTM label has been removed.

@roboquat roboquat removed the lgtm label Oct 14, 2021
Co-authored-by: George Tsiolis <tsiolis.g@gmail.com>
@jankeromnes
Copy link
Contributor Author

jankeromnes commented Oct 14, 2021

(The first build did not actually fail, and the result looks as expected.)

Carrying over the lgtm label, since the only changes were:

  • a rebase
  • applying the suggested CSS class change

And removing the block as well 🚀

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Oct 14, 2021

/werft run

👍 started the job as gitpod-build-jx-fix-project-card.4

@jankeromnes
Copy link
Contributor Author

jankeromnes commented Oct 14, 2021

Come on bitnami, let's try this again!

/werft run

👍 started the job as gitpod-build-jx-fix-project-card.5

@roboquat roboquat merged commit 0c4c212 into main Oct 14, 2021
@roboquat roboquat deleted the jx/fix-project-card branch October 14, 2021 06:52
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Oct 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/S team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Last prebuild timestamp is misaligned and breaks layout
3 participants