Skip to content

Added OnNewTag, allows to react to Return key in TagTextField, fixed fatal problem with landscape mode #1

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Vithanco
Copy link

@Vithanco Vithanco commented May 12, 2024

Added possibility to trigger action when tapped on tag.
Fixed an issue with geometry reader in landscape mode.

@Vithanco Vithanco changed the title Added OnNewTag, allows to react to Return key in TagTextField Added OnNewTag, allows to react to Return key in TagTextField, fixed fatal problem with landscape mode Jun 26, 2024
@danielsaidi
Copy link
Owner

Hi @Vithanco

Thank you for the PR.

To help me understand, why is the geometry reader thing needed? What are the problems in landscape? Please clarify, since I don't want to use GeometryReader unless explicitly needed.

In the tag list, the idea is that you can add everything you need to the view builder, so adding an additional action parameter should not be necessary.

The text field also should not need the action parameter, since you can just add the onSubmit to the tag text view. You do not need to specify that within that view.

@Vithanco
Copy link
Author

you are absolutely right, Daniel! I guess the way how you can use SwiftUI are still not internalised on my side. I removed tap gesture and on submit. Thanks!

I am not a fan of geometry reader neither. But please notice that I didn't introduce it, I only refactored your code. The problem that your code has is that it lead to a program abortion, and the updated version seems to work (and hopefully is better to read).

@FireLord
Copy link

FireLord commented Nov 4, 2024

this fixes issue generated here #4

edit:
it doesnt

@danielsaidi
Copy link
Owner

Sorry for completely dropping the ball on this @Vithanco!

If this solves the problem with Swift 6, I will give it a try, then merge it and re-enable Swift 6 👍

@FireLord
Copy link

FireLord commented Nov 6, 2024

Sorry for completely dropping the ball on this @Vithanco!

If this solves the problem with Swift 6, I will give it a try, then merge it and re-enable Swift 6 👍

sorry i used his repo directly which didnt have swift 6 so maybe thats why it worked. Ill check his commit with swift 6 and let you know!

@danielsaidi
Copy link
Owner

danielsaidi commented Nov 6, 2024

sorry i used his repo directly which didnt have swift 6 so maybe thats why it worked. Ill check his commit with swift 6 and let you know!

Thank you!

@FireLord
Copy link

FireLord commented Nov 6, 2024

sorry i used his repo directly which didnt have swift 6 so maybe thats why it worked. Ill check his commit with swift 6 and let you know!

Thank you!

this pr doesnt fix that error. But your latest 0.4.1 with swift 5.9 builds fine!

@danielsaidi danielsaidi force-pushed the main branch 2 times, most recently from 6c96b83 to b559c4a Compare April 3, 2025 12:47
@danielsaidi
Copy link
Owner

Hi @Vithanco

I have updated the library for Swift 6, and simplified many views.

Does this still happen?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants