Skip to content
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

Improve ContactList layout and improve subtitle line breaking #18

Merged
merged 3 commits into from Nov 5, 2023

Conversation

Supereg
Copy link
Member

@Supereg Supereg commented Nov 2, 2023

Improve ContactList layout and improve subtitle line breaking

♻️ Current situation & Problem

Currently, the ContactsList view is built using a custom ScrollView implementation and drawing custom backgrounds with shades around the ContactViews. This doesn't really feel native on iOS. Instead, this PR rethinks the implementation by rebuilding the ContactsList view using SwiftUI standard components like List. This makes the ContactsList now feel right at home.
Further, previously the subtitle, consisting of the Person's title and organization, was built using multiple distinct Text instances. This caused problems when the text was longer than the view was capable of displaying. This PR ensures that a persons title and organization are combined into a single Text view and allows to to wrap into a second line.
Lastly, this PR migrates the PR to use String catalogs, optimizes key naming and bumps the target to iOS 17.

Below are two screenshots comparing the previous implementation to the updated one.

Bildschirmfoto 2023-11-01 um 22 13 11 Bildschirmfoto 2023-11-01 um 22 12 14

⚙️ Release Notes

  • New updated visuals for the ContactsView that feel way more native to iOS
  • Updated line breaking behavior for too long contact subtitles (title and organization)
  • Migrated to use String Catalogs for localization

📚 Documentation

--

✅ Testing

Tests were slightly adjusted, due to issues with line breaks in the labels.

📝 Code of Conduct & Contributing Guidelines

By submitting creating this pull request, you agree to follow our Code of Conduct and Contributing Guidelines:

Copy link

codecov bot commented Nov 2, 2023

Codecov Report

Merging #18 (fc72985) into main (9dea670) will increase coverage by 1.17%.
The diff coverage is 93.75%.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main      #18      +/-   ##
==========================================
+ Coverage   80.07%   81.23%   +1.17%     
==========================================
  Files           5        4       -1     
  Lines         311      309       -2     
==========================================
+ Hits          249      251       +2     
+ Misses         62       58       -4     
Files Coverage Δ
...urces/SpeziContact/Contact Views/ContactView.swift 82.36% <100.00%> (+1.01%) ⬆️
Sources/SpeziContact/Models/Contact.swift 100.00% <ø> (ø)
...rces/SpeziContact/Contact Views/ContactsList.swift 50.00% <83.34%> (-1.85%) ⬇️
Sources/SpeziContact/Models/ContactOption.swift 86.96% <85.72%> (+0.81%) ⬆️

Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9dea670...fc72985. Read the comment docs.

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

Thank you for the great improvements; this looks great @Supereg! 🚀

@Supereg
Copy link
Member Author

Supereg commented Nov 2, 2023

Will tag the new SpeziViews later on and still include it in this PR.

@vishnuravi
Copy link
Member

This looks much better! Thank you @Supereg!

Copy link
Member

@PSchmiedmayer PSchmiedmayer left a comment

Choose a reason for hiding this comment

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

👍🚀

@Supereg Supereg merged commit bd38bd7 into main Nov 5, 2023
7 checks passed
@Supereg Supereg deleted the feature/adjust-list-wrapping-string-catalogs branch November 5, 2023 23:02
@Supereg Supereg mentioned this pull request Nov 17, 2023
1 task
Supereg added a commit that referenced this pull request Nov 17, 2023
# Fix dark mode appearance

## ♻️ Current situation & Problem
This PR fixes appearance in dark mode. Due to refactorings to lists in
#18, we know have to adapt to different backgrounds (e.g., placed
directly into a view, placed into a grouped background view, like a List
cell).
This was primarily an issue as we used a background color instead of a
fill color (see
[tertiarySystemFill](https://developer.apple.com/documentation/uikit/uicolor/3255076-tertiarysystemfill)
being recommended for buttons and compare to
[secondarySystemBackground](https://developer.apple.com/documentation/uikit/uicolor/3173137-secondarysystembackground)
which we previously used).


## ⚙️ Release Notes 
* Fixes appearance in light background when appearing in Lists.


## 📚 Documentation
--


## ✅ Testing
--


## 📝 Code of Conduct & Contributing Guidelines 

By submitting creating this pull request, you agree to follow our [Code
of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md):
- [x] I agree to follow the [Code of
Conduct](https://github.com/StanfordSpezi/.github/blob/main/CODE_OF_CONDUCT.md)
and [Contributing
Guidelines](https://github.com/StanfordSpezi/.github/blob/main/CONTRIBUTING.md).
@vishnuravi vishnuravi mentioned this pull request Nov 20, 2023
1 task
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.

None yet

3 participants