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

[feature/avatar] Avatar and generalized resource support #1092

Closed
wants to merge 4 commits into from

Conversation

felix-schwarz
Copy link
Contributor

@felix-schwarz felix-schwarz commented Jan 21, 2022

Description

Support for avatars, with accompanying minor design tweaks.

Builds upon the SDK's new generalized resource management system that provides a mechanism for retrieving, caching and updating resources such as item thumbnails and avatars.

Related Issue

#408

Related PRs

owncloud/ios-sdk#81

Screenshots (if appropriate):

Single Account View

Dark theme (before) and light theme (after). Avatar support is also available in dark theme, of course.

Before With Placeholder With Avatar
Simulator Screen Shot - iPhone 12 - 2022-01-21 at 10 57 24 Simulator Screen Shot - iPhone 12 - 2022-01-21 at 00 11 54 Simulator Screen Shot - iPhone 12 - 2022-01-21 at 00 12 11

Multi Account View

Before With Avatar support
Simulator Screen Shot - iPhone 12 mini - 2022-01-21 at 12 03 32 Simulator Screen Shot - iPhone 12 mini - 2022-01-21 at 12 02 04

Recipients search

Before With Avatar support
Simulator Screen Shot - iPhone 12 - 2022-01-21 at 10 58 18 Simulator Screen Shot - iPhone 12 - 2022-01-21 at 11 36 49

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation.
  • Added an issue with details about all relevant changes in the iOS documentation repository.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • Added changelog files for the fixed issues in folder changelog/unreleased

	- add OCViewHost as container for views provided via OCViewProviders / OCResourceRequests
	- add view providers for OCAvatar, OCResourceTextPlaceholder
	- add OCCircularContentView, OCCircularImageView, OCCircularTextView
- BookmarkViewController: download a users's avatar and save it in the bookmark
- ClientRootViewController: check for an updated version of the user's avatar on every connect
- GroupSharingTableViewController: display avatars instead of icons for users
- StaticTableViewRow: add support for leading accessory cell
- ThemeTableViewCell: extend support for custom layout, revise CellLayouter to support single line of text
- remove bolted-on QuickLook thumbnailing (now covered by the new OCResource system)
- update static/single login + accounts list UI to work with avatars
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@felix-schwarz felix-schwarz added this to the 11.10.0-Next milestone Jan 21, 2022
@felix-schwarz felix-schwarz linked an issue Jan 21, 2022 that may be closed by this pull request
…ager

- OCViewHost: improve responsiveness and direct updates (saving unnecessary main runloop cycles), add explicit reload method
- ResourceViewHost: subclass of OCViewHost with support for auto-reload on Theme change
- ItemPolicyCell, DisplayViewController, ServerListBookmarkCell, ShareClientItemCell, ClientItemCell, NamingViewController, MoreViewHeader: switch from UIImageView and old thumbnail / icon API mix to ResourceViewHost
- move ThemeView base class from ownCloudApp to ownCloudAppShared
- extend OCAvatar+ViewProvider to OCImage+ViewProvider, so it also returns views for all other OCImages
- add ResourceItemIcon and ResourceSourceItemIcons to return TVG resources and provide views for them
- VectorImageView: clear layer if not image could be created
@felix-schwarz felix-schwarz requested review from hosy and jesmrec January 24, 2022 21:38

attributedTitle.append(NSAttributedString(string: displayName.appendingFormat("\n"), attributes: [
NSAttributedString.Key.font : UIFont.boldSystemFont(ofSize: 24)
]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Font size could be set to .title2 (or similar) instead of using float value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Addressed this in 3036f54 in feature/graph-api.

if let serverName = bookmark.url?.host {
attributedTitle.append(NSAttributedString(string: serverName, attributes: [
NSAttributedString.Key.font : UIFont.systemFont(ofSize: 18)
]))
Copy link
Collaborator

Choose a reason for hiding this comment

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

Font size could be set to . headline (or similar) instead of using float value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks. Addressed this in 3036f54 in feature/graph-api.

@hosy
Copy link
Collaborator

hosy commented Apr 13, 2022

@felix-schwarz I left two small comments, but overall it is an awesome implementation. I love it! Please can you also update the branch and set the correct merge branch?

felix-schwarz added a commit that referenced this pull request Apr 25, 2022
@felix-schwarz
Copy link
Contributor Author

@hosy Thanks! I've addressed the findings in 3036f54 using a new UIFont method (added by category) in feature/graph-api that allows to pick a font weight in addition to the text style.

I'm now closing this PR and setting it as base for feature/graph-api.

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.

User Avatar
3 participants