-
-
Notifications
You must be signed in to change notification settings - Fork 8
Added powered by giphy attribution mark #43
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
base: main
Are you sure you want to change the base?
Conversation
When moving from beta to production key with the GIPHY API, you are required to display an attribution mark "Powered By Giphy" It is not that clear where you need to display this: This pull request adds the attribution mark to the giphy popup, and is fairly inconspicuous, it is off by default as the app developer may already be displaying the attribution mark somewhere else in the app. Basically I have had to add this to get through the review process to move from beta to production key |
@jaesung-0o0 Hi could you have a look at this pull request, many thanks Fred |
// Targets are the basic building blocks of a package. A target can define a module or a test suite. | ||
// Targets can depend on other targets in this package, and on products in packages this package depends on. | ||
.target( | ||
name: "ChatUI", | ||
dependencies: [ | ||
.product(name: "GiphyUISDK", package: "giphy-ios-sdk") | ||
], | ||
resources: [ | ||
.process("Resources") | ||
] | ||
), | ||
.testTarget( | ||
name: "ChatUITests", | ||
dependencies: ["ChatUI"]), | ||
] |
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.
// Targets are the basic building blocks of a package. A target can define a module or a test suite. | |
// Targets can depend on other targets in this package, and on products in packages this package depends on. | |
.target( | |
name: "ChatUI", | |
dependencies: [ | |
.product(name: "GiphyUISDK", package: "giphy-ios-sdk") | |
], | |
resources: [ | |
.process("Resources") | |
] | |
), | |
.testTarget( | |
name: "ChatUITests", | |
dependencies: ["ChatUI"]), | |
] | |
// Targets are the basic building blocks of a package. A target can define a module or a test suite. | |
// Targets can depend on other targets in this package, and on products in packages this package depends on. | |
.target( | |
name: "ChatUI", | |
dependencies: [ | |
.product(name: "GiphyUISDK", package: "giphy-ios-sdk") | |
], | |
resources: [ | |
.process("Resources") | |
] | |
), | |
.testTarget( | |
name: "ChatUITests", | |
dependencies: ["ChatUI"]), | |
] |
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.
Convention: Indent -> 4 space
|
||
struct GiphyAttributionMarkView: View { | ||
|
||
private let PADDING_ABOVE_TAB = 2.5 |
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.
private let PADDING_ABOVE_TAB = 2.5 | |
private let paddingAboveTab = 2.5 |
please modify snake case to lower camel case :)
: "Poweredby_100px-White_VertText" | ||
} | ||
} | ||
|
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.
Please add GiphyAttributionMarkView.Previews
to Previews/ChatInChannel
path:
struct GiphyAttributionMarkView_Preview: PreviewProvider {
static var previews: some View {
GiphyAttributionMarkView()
.preferredColorScheme(.dark)
}
}
Hi @fred-bowker :) Sorry for the late check. What a good catch! However, while reviewing the PR, I faced an image unassigned issue and the image doesn't show up on the preview canvas: Also could you add the previews of the new view: ![]() |
No description provided.