-
Notifications
You must be signed in to change notification settings - Fork 2
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
implement project cards hover and modal #45
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks like a great start! 🚀 just a thought: if we go with the transparent background modal, should we just make it full-screen so that the closing x
isn't floating in what seems like the middle of the screen?
public/index.html
Outdated
@@ -8,6 +8,7 @@ | |||
|
|||
<!-- Lato regular, regular italic --> | |||
<link href="https://fonts.googleapis.com/css2?family=Lato:ital@0;1&display=swap" rel="stylesheet"> | |||
<link rel="stylesheet" href="https://unpkg.com/bulma-modal-fx/dist/css/modal-fx.min.css" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What does this do? I took a quick look and it seems like bulma-modal-fx
's effects can be achieved with animate.css
, which we already depend on - our third-party CSS bundle is already quite large, let's try to avoid adding additional dependencies unless absolutely necessary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'll remove it. I am not actually using it, I added it when I was playing around with stuff
@@ -0,0 +1,5 @@ | |||
<?xml version="1.0" ?><!DOCTYPE svg PUBLIC '-//W3C//DTD SVG 1.1//EN' 'http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd'><svg height="100mm" style="shape-rendering:geometricPrecision; text-rendering:geometricPrecision; image-rendering:optimizeQuality; fill-rule:evenodd; clip-rule:evenodd" viewBox="0 0 100 100" width="100mm" xml:space="preserve" xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink"><defs><style type="text/css"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this doesn't quite seem like the GitHub icon from the old side, which I think might look a bit better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if we should just use fontawesome
- see #46
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use fontawesome
Yeah that works and the original intention was a solid background for the modal. I was playing around and by default bulma does modals as transparent. Now that I look at it it doesn't look too shabby. I know in general most modals have a solid background but in this case I think the text readability isn't affected. However, I can try one with the background and see whether it works better. Here it is: |
…e or no image, made modal background solid
I like the hover behaviour for images without a title! Thought: a gentler glow/light drop shadow would probably look better on hover The website is dark, but something reminiscent of one of @cowjuh 's showcase designs would be nice imo: |
@RachitMalik12 looks better! maybe add a little more spread to the glow? |
Sorry for the nit, but still unsure about how 5bd8cad looks 😅 I think we should go for an even spread around the tile since it's not a real "shadow" and should be more of a uniform glow |
…ture/project-cards-enhancement
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
imo this is a good start! I think we can make additional improvements down the line.
I just have one last nit: it seems in the on-hover for projects to show names, the name itself also gets the transparency applied, when it should be completely opaque:
if that proves difficult to fix, I don't mind holding off on it for now. let's get an approve from @srijonsaha as well before we merge?
Yeah it bugs me too. I'll try to fix it.
Sure! Thanks! |
@RachitMalik12 just merged #61 - let's see how the new config looks with this branch? |
…ture/project-cards-enhancement
@bobheadxi |
A lot better, thank you! |
What's done:
Screenshots of what's done
Transparent:
Background:
Animations:
What's left:
Animation of cards and edge case where banner doesn't have the nameAlign the github logo and get better assets that go well with the background