8000 Sticker Packs by Sjmarf · Pull Request #150 · SwiftcordApp/Swiftcord · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Sticker Packs #150

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
Merged
merged 16 commits into from
May 16, 2023
Merged

Sticker Packs #150

merged 16 commits into from
May 16, 2023

Conversation

Sjmarf
Copy link
Contributor
@Sjmarf Sjmarf commented May 13, 2023 •
edited
Loading

Warning
This relies on DiscordKit being updated to include my recent PR.

  • Changed Stickers to use Buttons for click detection instead of .onTapGesture. This means that the Bird sticker, which previously wouldn't show a preview when clicked, now works correctly.
  • Added a Sticker Pack view that can be accessed from the sticker preview view. This can be a little laggy on the larger sticker packs, but it's usable (on my device).
  • Moved the code for hover detection from the sticker-view used in messages to the basic StickerItemView. This is because the hover-detection code is reused in the sticker pack view.
  • Renamed StickerView (the view used to display stickers in messages) to MessageStickerView.

cryptoAlgorithm
cryptoAlgorithm requested changes May 14, 2023
View reviewed changes
Copy link
Member
@cryptoAlgorithm cryptoAlgorithm left a comment

Choose a reason for hiding this comment

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

Overall, this PR was high quality, but there were some issues with changes to the project file and some other lint warnings. Applying all suggested fixes should resolve issues to the project file, but lint errors need to be manually resolved. Swiftcord's codebase has switched to only using spaces for indentation, so I suggest the same be done here to prevent mixed indentation warnings.

Sjmarf and others added 4 commits May 14, 2023 09:13
@Sjmarf @cryptoAlgorithm
Update Swiftcord.xcodeproj/project.pbxproj
1f806c1 
Co-authored-by: CryptoAlgo <64193267+cryptoAlgorithm@users.noreply.github.com>
8000 @Sjmarf @cryptoAlgorithm
Update Swiftcord.xcodeproj/project.pbxproj
0086639 
Co-authored-by: CryptoAlgo <64193267+cryptoAlgorithm@users.noreply.github.com>
@Sjmarf @cryptoAlgorithm
Update Swiftcord.xcodeproj/project.pbxproj
cd8f082 
Co-authored-by: CryptoAlgo <64193267+cryptoAlgorithm@users.noreply.github.com>
@Sjmarf @cryptoAlgorithm
Update Swiftcord.xcodeproj/project.pbxproj
c1723ac 
Co-authored-by: CryptoAlgo <64193267+cryptoAlgorithm@users.noreply.github.com>
@cryptoAlgorithm
Copy link
Member

I'd have preferred if all suggested changes were batched into a single commit, but I guess this works too as it would be eventually squashed for merging.

Sjmarf and others added 3 commits May 14, 2023 09:14
@Sjmarf @cryptoAlgorithm
Update Swiftcord/Views/Message/MessageStickerView.swift
159f1c9 
Co-authored-by: CryptoAlgo <64193267+cryptoAlgorithm@users.noreply.github.com>
@Sjmarf
Linting
584d789
@Sjmarf
Merge remote-tracking branch 'refs/remotes/origin/main'
de1e9f9 
Conflicts:
	Swiftcord/Views/Message/MessageStickerView.swift
@Sjmarf
Copy link
Contributor Author
Sjmarf commented May 14, 2023

It should be good now, I think 👍

cryptoAlgorithm
cryptoAlgorithm requested changes May 14, 2023
View reviewed changes
@cryptoAlgorithm
Copy link
Member

It should be good now, I think 👍

Nice, there doesn't appear to be any more lint warnings/errors remaining! However, there still are some references to a local copy of DiscordKit, which shouldn't be committed.

@cryptoAlgorithm
Copy link
Member

Ok, one final issue: looks like the commit hash of DiscordKit in Package.resolved is still pointing to an old commit, it needs to point to at least the commit that adds the required symbols, else building on CI will not succeed. You can follow these instructions #150 (comment) to update the dependancy reference.

@Sjmarf
Copy link
Contributor Author
Sjmarf commented May 14, 2023

Ok, one final issue: looks like the commit hash of DiscordKit in Package.resolved is still pointing to an old commit, it needs to point to at least the commit that adds the required symbols, else building on CI will not succeed. You can follow these instructions #150 (comment) to update the dependancy reference.

Hopefully it's working now 😅

@cryptoAlgorithm cryptoAlgorithm added this pull request to the merge queue May 16, 2023
Merged via the queue into SwiftcordApp:main with commit 7651c43 May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cryptoAlgorithm cryptoAlgorithm cryptoAlgorithm requested changes

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

Successfully merging this pull request may close these issues.
2 participants
@Sjmarf @cryptoAlgorithm
0