profile settings #322
profile settings #322
Conversation
kloenk
force-pushed
the
matrixcore-profile-settings
branch
from
9f087fa to
dd89f70
Compare
|
|
||
| var body: some View { | ||
| List { | ||
| Section(header: Text("USER SETTINGS")) { |
There was a problem hiding this comment.
The heading should upper case this by itself I believe. Also if you're targeting iOS 15 (not sure) you can simply pass the string. Same comment for all the headers :)
| Section(header: Text("USER SETTINGS")) { | |
| Section("User Settings") { |
| } | ||
|
|
||
| // Password | ||
| Button("Change password", role: .destructive) { |
There was a problem hiding this comment.
I wouldn't mark this as destructive myself, if something bad is going to happen it should show an alert explaining this instead, otherwise changing password should probably feel like a safe operation to the user.
| let nioAccount = store.accounts[account.userID!]! | ||
|
|
||
| if let displayName = changes["displayName"] as? String { | ||
| NioAccountStore.logger.debug("Changing displayName to \(displayName)") |
There was a problem hiding this comment.
Is it necessary to actually log the new displayname? Always seems like a good habit to me to avoid logging personal data where possible. (Unless Logger redacts this string interpolation by default, in which case ignore me as I haven't used it in a while).
| .navigationBarTitleDisplayMode(.inline) | ||
| .toolbar { | ||
| ToolbarItem { | ||
| Button(action: { |
There was a problem hiding this comment.
As the label is simply text label, it's probably neater to
Button("Save") {
// do stuff
}
|
|
||
| // TODO: verified/unverified devices sections? | ||
| Section(header: Text("Devices")) { | ||
| ForEach(devices, id: \.deviceID) { device in |
There was a problem hiding this comment.
Side note: We should probably conform MatrixDevice to Identifiable.
| .refreshable { | ||
| await self.updateDevices() | ||
| } | ||
| .toolbar { |
There was a problem hiding this comment.
Personally I'd consolidate the 2 toolbar modifiers into 1, but I can see why you might prefer 2 :)
NioUIKit/ProfileSettings/Security/Devices/ProfileSettingsSecurityDevicesContainerView.swift
Show resolved
Hide resolved
| private func delete(at offsets: IndexSet) { | ||
| let idsToDelete = offsets.map { self.devices[$0].deviceID } | ||
|
|
||
| _ = idsToDelete.compactMap { id in |
There was a problem hiding this comment.
| _ = idsToDelete.compactMap { id in | |
| idsToDelete.forEach { id in |
| struct ProfileSettingsSecurityDeviceView: View { | ||
| let device: MatrixDevice | ||
|
|
||
| let subText: String |
There was a problem hiding this comment.
| let subText: String | |
| let subtext: String |
| Button(action: { | ||
| self.setDisplayName() | ||
| }) { | ||
| Text("Save") | ||
| } |
There was a problem hiding this comment.
| Button(action: { | |
| self.setDisplayName() | |
| }) { | |
| Text("Save") | |
| } | |
| Button("Save") { | |
| self.setDisplayName() | |
| } |
| if editMode == .active { | ||
| Button("Cancel") { | ||
| self.selection.removeAll() | ||
| withAnimation { | ||
| self.editMode = .inactive | ||
| } | ||
| } | ||
| } else { | ||
| Button("Edit") { | ||
| withAnimation { | ||
| self.editMode = .active | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Where possible, it's useful to update the parameters on a view rather than switching out the view for a new one:
| if editMode == .active { | |
| Button("Cancel") { | |
| self.selection.removeAll() | |
| withAnimation { | |
| self.editMode = .inactive | |
| } | |
| } | |
| } else { | |
| Button("Edit") { | |
| withAnimation { | |
| self.editMode = .active | |
| } | |
| } | |
| } | |
| Button(editMode.isEditing ? "Cancel" : "Edit") { | |
| if editMode.isEditing { | |
| self.selection.removeAll() | |
| withAnimation { | |
| self.editMode = .inactive | |
| } | |
| } else { | |
| withAnimation { | |
| self.editMode = .active | |
| } | |
| } | |
| } |
No description provided.