Skip to content

Conversation

@nobbes
Copy link
Contributor

@nobbes nobbes commented May 23, 2023

Added: share Link, copy Link in QR code view, translation
Update: dependencies, unit test
Remove: warnings, bugs :)

@Hustenbonbon
Copy link
Collaborator

Hi, thanks for all the contribution, unfortunately currently I am very engaged in other projects and got little time to review and test all the small changes in detail.
If you plan to contribute more often, feel free to contact me on the mail in my github profile. We have a matrix chatroom with the developers of Cospend, iHateMoney, the Android and this app

@Hustenbonbon
Copy link
Collaborator

Feel free to separate the localization, dispatchqueues and test-fixes in separate PRs, I can merge those right away

@nobbes
Copy link
Contributor Author

nobbes commented May 24, 2023

Thank you for your response, I will keep this in mind.

InteractionEngineer

This comment was marked as resolved.

Copy link
Owner

@InteractionEngineer InteractionEngineer left a comment

Choose a reason for hiding this comment

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

First of all: Thank you for your contribution – I like the features you added to make it easier to share projects with others! Please look into the comments I left on each file modified. After the necessary changes where made, I'm happy to merge your PR and bring your feature to all current users.

Choose a reason for hiding this comment

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

outdated, GRDB is up to date now and two major versions ahead.

Choose a reason for hiding this comment

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

As this is only for preview, and preview works without initializing this struct (again), better remove it completely.

Choose a reason for hiding this comment

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

'animation' was deprecated in iOS 15.0 – so it would be even better to use 'withAnimation'. Your fix works too, though – but let's make the switch sooner than later.

Choose a reason for hiding this comment

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

'animation' was deprecated in iOS 15.0 – so it would be even better to use 'withAnimation'. Your fix works too, though – but let's make the switch sooner than later.

Choose a reason for hiding this comment

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

super helpful feature, thank you! It's best practice to keep logic and interface separated. Would you mind "outsourcing" this feature? This way it can easily be reused in different places and even tested automatically (more easily).

Choose a reason for hiding this comment

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

outdated but correct! …at this time already in the codebase

@InteractionEngineer InteractionEngineer added enhancement New feature or request question Further information is requested labels May 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request question Further information is requested

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants