Skip to content

Conversation

@mpivchev
Copy link
Collaborator

@mpivchev mpivchev commented Jun 12, 2023

This adds a new section in settings to promote our other apps:

  • If the app is installed, it opens it without doing anything else.

  • If the app is not installed, it leads to the app store page.

  • If the "more apps" icon is clicked, it opens a web page with other Nextcloud apps.

Additionally, a new URL scheme was made to add support for other apps to open this app and switch accounts (if account is logged in):

nextcloud://open-and-switch-account?user=someUser&url=https://cloud.nextcloud.com

Finally, refactored some of the code and removed duplicate code.

Signed-off-by: Milen Pivchev <[email protected]>
mpivchev added 2 commits June 13, 2023 12:18
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
mpivchev added 2 commits June 15, 2023 12:29
Signed-off-by: Milen Pivchev <[email protected]>
Comment on lines -493 to -517
class CCCellMore: UITableViewCell {

@IBOutlet weak var labelText: UILabel!
@IBOutlet weak var imageIcon: UIImageView!
@IBOutlet weak var separator: UIView!
@IBOutlet weak var separatorHeigth: NSLayoutConstraint!

override var frame: CGRect {
get {
return super.frame
}
set (newFrame) {
var frame = newFrame
let newWidth = frame.width * 0.90
let space = (frame.width - newWidth) / 2
frame.size.width = newWidth
frame.origin.x += space
super.frame = frame
}
}
}

class NCMoreUserCell: UITableViewCell {

@IBOutlet weak var displayName: UILabel!
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These were separated in their own files

Comment on lines +11 to +35
class BaseNCMoreCell: UITableViewCell {
let selectionColor: UIView = UIView()
let defaultCornerRadius: CGFloat = 10.0

override var frame: CGRect {
get {
return super.frame
}
set (newFrame) {
var frame = newFrame
let newWidth = frame.width * 0.90
let space = (frame.width - newWidth) / 2
frame.size.width = newWidth
frame.origin.x += space
super.frame = frame
}
}

override func awakeFromNib() {
super.awakeFromNib()

selectedBackgroundView = selectionColor
backgroundColor = .secondarySystemGroupedBackground
layer.cornerRadius = defaultCornerRadius
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This code was used for every cell, so I made a base class instead.

@mpivchev mpivchev marked this pull request as ready for review June 15, 2023 11:09
Signed-off-by: Milen Pivchev <[email protected]>
@mpivchev mpivchev requested a review from jancborchardt June 15, 2023 11:13
@tobiasKaminsky
Copy link
Member

tobiasKaminsky commented Jun 16, 2023

Can you use

https://github.com/nextcloud/desktop/blob/d8479fb1bafacbef2ecdb26516b9c7f513204f01/theme/white/more-apps.svg?plain=1

for more icon?

Signed-off-by: Milen Pivchev <[email protected]>
@marinofaggiana
Copy link
Member

marinofaggiana commented Jun 16, 2023

we could take the opportunity to migrate this simple view into swiftUI and the GUI does not convince me

@mpivchev
Copy link
Collaborator Author

we could take the opportunity to migrate this simple view into swiftUI and the GUI does not convince me

Hmm, we could've decided to do this earlier before these changes were made. But I can try nonetheless.

the GUI does not convince me

Any suggestions are welcome. We can maybe ask some of our UI designers to design a view.

@marinofaggiana
Copy link
Member

we could take the opportunity to migrate this simple view into swiftUI and the GUI does not convince me

Hmm, we could've decided to do this earlier before these changes were made. But I can try nonetheless.

was only a consideration.

the GUI does not convince me

Any suggestions are welcome. We can maybe ask some of our UI designers to design a view.

I have to think about it.

@marinofaggiana marinofaggiana requested review from tobiasKaminsky and removed request for tobiasKaminsky June 18, 2023 09:20
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Nice! Design-wise, how about something standard like Apple also uses on the top of Contacts? That has proper spacing and all.

@mpivchev
Copy link
Collaborator Author

mpivchev commented Jun 21, 2023

@jancborchardt you mean this part?

image

Since we only have 3 of these, should they be stacked left or be centered?

@marinofaggiana
Copy link
Member

Haha I am on holiday and had saved this

image

@mpivchev mpivchev requested a review from jancborchardt June 21, 2023 18:01
Copy link
Member

@marcoambrosini marcoambrosini left a comment

Choose a reason for hiding this comment

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

I don't think that these shortcuts should be the first entry in the settings because they can be detrimental to usability and create friction. I would say that settings is probably the wrong place to advertise the presence of other apps, and I suggest something subtler and "dismissable". if we absolutely have to, what about moving them to the bottom of the list?

@mpivchev
Copy link
Collaborator Author

I don't think that these shortcuts should be the first entry in the settings because they can be detrimental to usability and create friction. I would say that settings is probably the wrong place to advertise the presence of other apps, and I suggest something subtler and "dismissable". if we absolutely have to, what about moving them to the bottom of the list?

I see your point. I still need to move this forward however, so if @jancborchardt is ok with this I can move this section to the bottom.

@jancborchardt
Copy link
Member

Since we only have 3 of these, should they be stacked left or be centered?

@mpivchev is there any recommended default or standard? Centered seems good.


@marcoambrosini the issue if they are on the bottom is that they will be difficult to discover, defeating the purpose. It is obvious that the vertical list of entries goes on, but it would not be obvious that there is an entirely new element below.

Signed-off-by: Milen Pivchev <[email protected]>
@mpivchev
Copy link
Collaborator Author

@jancborchardt Here is the new look:

@jancborchardt
Copy link
Member

In iOS Contacts, there is a bit more top and bottom padding inside the entries. Can you adjust that @mpivchev? :)

@mpivchev
Copy link
Collaborator Author

mpivchev commented Jun 22, 2023

@jancborchardt Is this better?

mpivchev added 5 commits June 22, 2023 16:12
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
…add-other-nextcloud-apps-to-settings-as-suggestions

Signed-off-by: Milen Pivchev <[email protected]>

# Conflicts:
#	Nextcloud.xcodeproj/project.pbxproj
#	iOSClient/NCGlobal.swift
Signed-off-by: Milen Pivchev <[email protected]>
Signed-off-by: Milen Pivchev <[email protected]>
@codecov
Copy link

codecov bot commented Jun 22, 2023

Codecov Report

Patch coverage: 0.98% and project coverage change: -0.20 ⚠️

Comparison is base (422440e) 9.03% compared to head (d604a3c) 8.84%.

❗ Current head d604a3c differs from pull request most recent head 9f54ed4. Consider uploading reports for the commit 9f54ed4 to get more accurate results

Additional details and impacted files
@@            Coverage Diff             @@
##           develop   #2461      +/-   ##
==========================================
- Coverage     9.03%   8.84%   -0.20%     
==========================================
  Files          185     188       +3     
  Lines        25747   25799      +52     
  Branches      9501    9529      +28     
==========================================
- Hits          2326    2281      -45     
- Misses       23217   23324     +107     
+ Partials       204     194      -10     
Impacted Files Coverage Δ
iOSClient/AppDelegate.swift 29.72% <0.00%> (-0.39%) ⬇️
iOSClient/More/Cells/BaseNCMoreCell.swift 0.00% <0.00%> (ø)
...OSClient/More/Cells/NCMoreAppSuggestionsCell.swift 0.00% <0.00%> (ø)
iOSClient/More/Cells/NCMoreUserCell.swift 0.00% <0.00%> (ø)
iOSClient/NCGlobal.swift 32.25% <ø> (ø)
iOSClient/More/NCMore.swift 2.21% <2.17%> (+0.34%) ⬆️

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@mpivchev
Copy link
Collaborator Author

@jancborchardt pinging again in case you missed it

@mpivchev mpivchev requested a review from marcoambrosini June 26, 2023 08:20
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Latest screenshot looks good! :)

@mpivchev mpivchev merged commit e66bcd6 into develop Jun 26, 2023
@delete-merged-branch delete-merged-branch bot deleted the 2460-add-other-nextcloud-apps-to-settings-as-suggestions branch June 26, 2023 10:22
@hannesfritz
Copy link

iOS seems to stretch those buttons if less are available. I just found an example with 4 instead of 5 like in the screenshot above. But I did an additional mockup to show what it might look with only the three options.
In my opinion it is calmer on the eye, better aligned and additionally larger tap targets.

So just food for thought :)
Contacts 4 jumps
Nextcloud more

@jancborchardt
Copy link
Member

Yeah, that seems very nice @hannesfritz!

@mpivchev could you adjust them like that? :)

@mpivchev mpivchev restored the 2460-add-other-nextcloud-apps-to-settings-as-suggestions branch July 5, 2023 12:19
@mpivchev
Copy link
Collaborator Author

mpivchev commented Jul 5, 2023

Branch restored so I can reimplement this soon

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.

Add other Nextcloud apps to settings as suggestions

7 participants