feat(leadspace): add leadspace brand logo style #309
Conversation
nimithshetty17
requested review from
proeung,
helms-charity,
keith-kaplan,
sachinmesh,
sharathmrft and
sahmad-merative
|
Hello, I'm Franklin Bot and I will run some test suites that validate the page speed.
|
|
|
|
@sachinmesh - Please have a look now. |
There was a problem hiding this comment.
@nimithshetty17
1.Video player is not playing the video
- There is variations in the Buttons size
|
HI @nimithshetty17 
|
|
@sachinmesh - Arrow is added as well. |
|
|
@proeung - Updated all the changes requested. Please let me know if there are any concerns. |
|
There was a problem hiding this comment.
@nimithshetty17 Your latest changes looks good, but I noticed that you were using a new arrow icon (arrow-right.svg) in the button. Just wanted to note that we already have an arrow icon SVG within the /icons directory within this repo (https://github.com/hlxsites/merative2/blob/main/icons/arrow.svg?short_path=f27ac62) and there's no need to upload a separate version.
Sharepoint format for primary button: Speak to sales:arrow:
Things to change/clean up:
- Remove this duplicate arrow SVG that you've uploaded (see link ).
- Push up a commit to adjust the existing
arrow.svgcode to use the following SVG code with the proper viewBox value of 24x24px for consistent scaling.
<svg width="24" height="24" viewBox="0 0 24 24" fill="none" xmlns="http://www.w3.org/2000/svg">
<path d="M3.75 12.6152H20.25" stroke="white" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round"/>
<path d="M13.5 5.86523L20.25 12.6152L13.5 19.3652" stroke="white" stroke-width="1.5" stroke-linecap="round" stroke-linejoin="round"/>
</svg>
Once, these changes are in, we can mark this PR as ready for merge.
cc: @keith-kaplan
|
|
@proeung - I have committed the changes for the arrow icon. Please let me know if there are any concerns. |
There was a problem hiding this comment.
@nimithshetty17 LGTM!
This PR is blocked from being merged until @sharathmrft has reviewed and approved it.
|
@nimithshetty17 @proeung
|
sachinmesh
added
the
Ready to merge
proeung
removed
the
Ready to merge
|
There was a problem hiding this comment.
@sharathmrft Thanks for the review. Yeah, I noticed the odd line height issue for the tertiary link on the mobile breakpoint as well. @nimithshetty17 Can you fix this?
|
|
|
@sharathmrft - Please review this now. |
|
There was a problem hiding this comment.
@nimithshetty17 Looks like you've added different line height values to the tertiary link for mobile (line-height-160) and desktop (line-height-120), which is incorrect.
I went a head and push up a commit (20d2f12) to fix and clean up the format of the code. Tertiary link line height is looking inconsistent on all breakpoints now. I'm going to mark and merge this PR down.
Issue
Fixes #MERATIVE-776
Description
We have a "Leadspace" block created in Franklin, however, we need to add a variation where the brand logo can be inserted into the block table within the Sharepoint docs.
New
Design Specs
Test URLs
main): https://main--merative2--nimithshetty17.hlx.page/block-library/blocks/leadspaceTesting Instruction
To test the leadspace with brand logo and its responsiveness with respect to the design across all devices.