This repository has been archived by the owner on Aug 13, 2023. It is now read-only.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Add scripLink prop to render
ScriptLink
component #2474Add scripLink prop to render
ScriptLink
component #2474Changes from all commits
6907262
4fd5ebe
de40040
d6e1bb3
b9eba95
090f539
eaaad91
0b58073
593ca97
c7c324a
bc2016d
bf87fb7
78fd442
49a951a
3764d8d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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.
The
top
calculation is wrong when the screen is small, making the link to go outside the banner when it should stay in it.![brand-wrong](https://user-images.githubusercontent.com/46446236/67426613-5d5faf80-f5d2-11e9-81c8-a91f8faf1fd8.png)
⬇Instead of making it absolute and calculate the top position, an alternative would be to add a new
div
to wrap both, Brand and Script Link, and use flex to align the items vertically and put them on each side. And then you wouldn't needScriptLinkWrapper
andSvgWrapper
, just to apply a couple of tweaks to theBanner
height and padding. Up to you :)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.
@DenisHdz Good point. Done 😎