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.

Sign up for GitHub

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

Jump to bottom

Change Zap automation UI issues #5355

Merged
merged 18 commits into from
Nov 6, 2022
Merged

Change Zap automation UI issues #5355

merged 18 commits into from
Nov 6, 2022

Conversation

om-ray
Copy link
Contributor

@om-ray om-ray commented Nov 3, 2022 •
edited
Loading

What does this PR do?

This PR changes the UI for Zap automation to match the Figma
Before
image

After
Screen Shot 2022-11-03 at 3 24 51 PM

Fixes #5324

Environment: Staging(main branch) / Production

Type of change

  • Bug fix (non-breaking change which fixes an issue)

@vercel
Copy link

vercel bot commented Nov 3, 2022

@om-ray is attempting to deploy a commit to the cal Team on Vercel.

A member of the Team first needs to authorize it.

@what-the-diff
Copy link

what-the-diff bot commented Nov 3, 2022

  • Changed the background color of AppSettings component to gray-100
  • Added a border around TemplateCard and changed its background color to white
  • Removed rounded corners from icon in TemplateCard, added margin on left side of card content, removed button's width constraint and moved it below the text description for mobile viewports

CarinaWolli
CarinaWolli requested changes Nov 4, 2022
View reviewed changes
Copy link
Member

@CarinaWolli CarinaWolli left a comment

Choose a reason for hiding this comment

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

Looks good overall 🙌🏻 Just found some minor design issues.
In addition to the comments below: If one template card has more lines of text than the other one, the buttons don't have the correct position:
Screenshot 2022-11-04 at 12 28 00

packages/app-store/zapier/components/TemplateCard.tsx Outdated Show resolved Hide resolved
@ciaranha
Copy link
Member

ciaranha commented Nov 4, 2022

Nothing to add on top of @CarinaWolli's feedback. Looks great 👍

@om-ray om-ray requested review from CarinaWolli and removed request for ciaranha November 4, 2022 16:50
@vercel
Copy link

vercel bot commented Nov 4, 2022 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
cal ❌ Failed (Inspect) Nov 6, 2022 at 4:04PM (UTC)

@CarinaWolli
Copy link
Member

If one template card has more lines of text than the other one, the buttons don't have the correct position:
Screenshot 2022-11-04 at 12 28 00

@om-ray This one is still open

This is how it looks, if the other template text is longer and has 2 rows:

Screenshot 2022-11-04 at 18 23 43

There is some additional gap at the top (above Gmail) and bottom (bellow the button).
That's how it looks for the one with the 2 rows (and also how it should look):

Screenshot 2022-11-04 at 18 23 34

@om-ray
Copy link
Contributor Author

om-ray commented Nov 4, 2022

@CarinaWolli Oh! i didn't notice that at all, I'll fix that asap!

@om-ray
Copy link
Contributor Author

om-ray commented Nov 4, 2022

@CarinaWolli It should be fixed now, thank you so much for your reviews!

CarinaWolli
CarinaWolli approved these changes Nov 4, 2022
View reviewed changes
Copy link
Member

@CarinaWolli CarinaWolli left a comment

Choose a reason for hiding this comment

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

Nice work! Thank you for contributing 🙏

@om-ray om-ray requested a review from PeerRich November 4, 2022 20:56
@PeerRich PeerRich enabled auto-merge (squash) November 4, 2022 21:15
@PeerRich PeerRich merged commit d9a867b into calcom:main Nov 6, 2022
haffla pushed a commit to tourlane/cal.com that referenced this pull request Nov 22, 2022
@om-ray @PeerRich @haffla
Change Zap automation UI issues (calcom#5355)
02b8fb1 
* changed styles for Zap automation ui to match figma

* Increased margintop value

* increase margintop value to mt-4

* fixed some styling issues

* fixed issue with buttons being at different heights

* changed vertical positioning of text and increased min height

Co-authored-by: Peer Richelsen <peeroke@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@CarinaWolli CarinaWolli CarinaWolli approved these changes

@PeerRich PeerRich Awaiting requested review from PeerRich

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[CAL-302] Installed Apps / Automation / Zapier - Zaps UI Issues
4 participants
@om-ray @ciaranha @CarinaWolli @PeerRich