careers page, various minor fixes #784 #786
Conversation
| const finalLayout = isMobile ? 'squeezed' : layout | ||
| className = `testimonial ${finalLayout} ${className || ''}` | ||
|
|
||
| return finalLayout === 'squeezed' ? ( |
There was a problem hiding this comment.
can't this be done with css + a media query?
There was a problem hiding this comment.
Yeah, that was what I thought at first, but the layouts are just different. Like the puzzle doesn't fit. I can't articulate why, just look at the different versions and you can see what I mean.
There was a problem hiding this comment.
that's unfortunate, but happens. @melissasamworth it's all your fault!!
| @@ -0,0 +1,650 @@ | |||
| import Card from '../Card' | |||
There was a problem hiding this comment.
you're going to have to move this to app/routes/how-can-i-help.career.tsx, which will be a bit of a problem - sorry about that
There was a problem hiding this comment.
All good, the change was needed. I've fixed it all now
| @@ -48,7 +48,7 @@ const Button = ({ | |||
| const LinkComponent = !action.startsWith('http') | |||
| ? Link | |||
| : ({to, children, ...props}: {[k: string]: any}) => ( | |||
| <a href={to} {...props}> | |||
| <a href={to} rel="noreferrer" target="_blank" {...props}> | |||
There was a problem hiding this comment.
@bryceerobertson I switched it here so that by default links to other pages open in a new tab. @mruwnik is that ok?
There was a problem hiding this comment.
not according to me. That's what the control button is for... But there was a whole discussion about this and I think the external links faction won :/ @melissasamworth is that correct?
| @@ -48,7 +48,7 @@ const Button = ({ | |||
| const LinkComponent = !action.startsWith('http') | |||
| ? Link | |||
| : ({to, children, ...props}: {[k: string]: any}) => ( | |||
| <a href={to} {...props}> | |||
| <a href={to} rel="noreferrer" target="_blank" {...props}> | |||
There was a problem hiding this comment.
not according to me. That's what the control button is for... But there was a whole discussion about this and I think the external links faction won :/ @melissasamworth is that correct?
| {links?.map((link) => <LinkCard key={link.title} action={link.action || '#'} {...link} />)} | ||
| {links?.map((link) => ( | ||
| <div key={link.title} className="padding-bottom-16"> | ||
| <LinkCard action={link.action || '#'} {...link} /> |
There was a problem hiding this comment.
why not apply the class directly to the LinkCard?
There was a problem hiding this comment.
Because we are spacing with padding, and the padding would go inside the button
| const finalLayout = isMobile ? 'squeezed' : layout | ||
| className = `testimonial ${finalLayout} ${className || ''}` | ||
|
|
||
| return finalLayout === 'squeezed' ? ( |
There was a problem hiding this comment.
that's unfortunate, but happens. @melissasamworth it's all your fault!!
No description provided.