feat: [CAL-3939] fix org banner #15557
Conversation
|
@panzacoder is attempting to deploy a commit to the cal Team on Vercel. A member of the Team first needs to authorize it. |
|
|
|
Hey there and thank you for opening this pull request! 👋🏼 We require pull request titles to follow the Conventional Commits specification and it looks like your proposed title needs to be adjusted. Details: |
github-actions
bot
added
Low priority
dosubot
bot
added
enterprise
panzacoder
changed the title
Graphite Automations"Add community label" took an action on this PR • (06/25/24)1 label was added to this PR based on Keith Williams's automation. "Add consumer team as reviewer" took an action on this PR • (06/25/24)1 reviewer was added to this PR based on Keith Williams's automation. |
There was a problem hiding this comment.
Small changes here to the ee/organizations setup process that were incorrect.
| @@ -33,6 +33,8 @@ import { | |||
| SkeletonText, | |||
| TextField, | |||
| } from "@calcom/ui"; | |||
| // if I include this in the above barrel import, I get a runtime error that the component is not exported. | |||
| import { OrgBanner } from "@calcom/ui"; | |||
There was a problem hiding this comment.
As code comment says, I was getting runtime errors if I didn't import this separately.
| @@ -93,7 +95,7 @@ const OrgProfileView = () => { | |||
| router.replace("/enterprise"); | |||
| } | |||
| }, | |||
| [error] | |||
| [error, router] | |||
There was a problem hiding this comment.
useEffect was complaining about missing this dep, so I added it.
| }; | ||
|
|
||
| export function OrgBanner(props: OrgBannerProps) { | ||
| const { imageSrc, alt, width = 1500, height = 500 } = props; |
There was a problem hiding this comment.
default width and height that is enforced elsewhere for OrgBanner
There was a problem hiding this comment.
@PeerRich the design in figma is different #13998 (comment)
Unfortunate that designs weren't attached to the ticket #15456 since they've been available for so long. This PR is based on the AC for that ticket, which is somewhat sparse. |
Sorry for the delay, but the tests and types are all good now, and I added a negative test as well. |
|
@Udit-takkar lets merge this, i think this is fine. this is on me, i didnt know we had a figma design for this. this is good enough, we can make the redesign later |
📦 Next.js Bundle Analysis for @calcom/webThis analysis was generated by the Next.js Bundle Analysis action. 🤖 Eighty-four Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the The "Budget %" column shows what percentage of your performance budget the First Load total takes up. For example, if your budget was 100kb, and a given page's first load size was 10kb, it would be 10% of your budget. You can also see how much this has increased or decreased compared to the base branch of your PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. If you see "+/- <0.01%" it means that there was a change in bundle size, but it is a trivial enough amount that it can be ignored. |
* update documentation for proper hostile command * add OrgBanner component and replace it in ee organization profile settings * add width and height defaults, with option to override * updated sizing with image loaded * fix: add alt to OrgBanner in test * fix: failing OrgBanner test, added neg test --------- Co-authored-by: Keith Williams <[email protected]> Co-authored-by: Anik Dhabal Babu <[email protected]> Co-authored-by: Udit Takkar <[email protected]> Co-authored-by: Peer Richelsen <[email protected]>
What does this PR do?
I introduced a new component in @calcom/ui to be used for previews of the org banner. This component has minimal functionality, aside for providing a placeholder/fallback that will match the size that is set (in css) for the image using next/image.
Loom Video: https://www.loom.com/share/af57febbb0484ff785d7a341ea5cbd1f?sid=81f73775-cb53-4938-9546-c942e8178a1d
Mandatory Tasks (DO NOT REMOVE)
How should this be tested?
acme.cal.local:3000(or whatever org you have setup)/settings/organizations/profileto see and test the component