-
-
Notifications
You must be signed in to change notification settings - Fork 680
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
style: reduce company logo size on casestudy/[id] page #2728
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-2728--asyncapi-website.netlify.app/ |
Also, would request you to wait for a triager to approve the issue before starting to work on it :) |
Approve? you mean 'to assign'? |
@RamGoel It doesn't look good in mobile view and IMO we don't need a change like this |
@RamGoel we can decrease the size of the company image which will look better |
yes sure. |
@sambhavgupta0705 I decreased the image size from |
Hey guys! Could you please review this. |
@sambhavgupta0705 as @anshgoyalevil suggested that the indentation earlier was not looking good. so I added this Here is updated indentation, this is same as we have earlier, that is why you aren't able to notice |
pages/casestudies/[id].js
Outdated
const typeStyle = | ||
level === 0 ? "heading-lg" : level === 1 ? "heading-md" : "heading-sm"; | ||
|
||
return content.map((item) => { | ||
return ( | ||
<div | ||
className="mt-10" | ||
className={`mt-10 mx-auto ${!isAgain?'w-11/12':''}`} |
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.
@RamGoel I am not able to understand the use of isAgain
So please remove it
@RamGoel just decrease the size of the image for this PR |
pages/casestudies/[id].js
Outdated
@@ -17,7 +17,7 @@ const renderContent = (content, allComponents, level) => { | |||
return content.map((item) => { | |||
return ( | |||
<div | |||
className="mt-10" | |||
className={`mt-10 mx-auto w-11/12`} |
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.
Rewrite this in conventional tailwind syntax as it was written previously
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.
you want to remove mx-auto w-11/12
as well? or just remove {} as there is no condition checking anymore?
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.
Just the braces
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.
LGTM cc: @anshgoyalevil
@RamGoel you have made the changes in indentation again |
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.
@RamGoel please revert the indentation change
The whole idea of this PR is fixing the spacing, so I used That is why I had added the |
@RamGoel in this PR the image size has been decreased correctly but the indentation of the points has been changes which we don't want Please revert this change |
/rtm |
Description
/casestudies/[id]
page and replaces any fixed width with relative width in tailwind classes.Related issue(s)
Resolves #2727
Preview