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

iOS Profile Settings and Advanced #3135

Merged
merged 18 commits into from
Feb 14, 2025

Conversation

reabbotted
Copy link

  • "Profile" page in the account
  • "Advanced" page in the account
  • TextInput HorizonUI component
  • SingleSelect HorizonUI component

Simulator Screenshot - iPhone 16 Pro Max - 2025-02-11 at 14 50 11
Simulator Screenshot - iPhone 16 Pro Max - 2025-02-11 at 14 50 01
Simulator Screenshot - iPhone 16 Pro Max - 2025-02-11 at 14 50 07

[ignore-commit-lint]

@inst-danger
Copy link
Contributor

inst-danger commented Feb 11, 2025

Horizon Build QR Code:

@inst-danger
Copy link
Contributor

inst-danger commented Feb 11, 2025

Fails
🚫 Build failed, skipping coverage check
❌ XCTest failed: CoreTests/WKHTTPCookieStoreExtensionsTests/testDeleteAllCookies
Asynchronous wait failed: Exceeded timeout of 0.1 seconds, with unfulfilled expectations: "Publisher finished".
XCTAssertEqual failed: ("[<NSHTTPCookie
	version:1
	name:testName
	value:testValue
	expiresDate:'(null)'
	created:'2025-02-13 18:02:31 +0000'
	sessionOnly:TRUE
	domain:instructure.com
	partition:none
	sameSite:none
	path:/login
	isSecure:FALSE
 path:"/login" isSecure:FALSE>]") is not equal to ("[]")
❌ XCTest failed: TeacherTests/PostGradesPresenterTests/testAllGradesPosted
Asynchronous wait failed: Exceeded timeout of 0.5 seconds, with unfulfilled expectations: "expectation".
XCTAssertTrue failed

Generated by 🚫 dangerJS against 0804138

Copy link
Contributor

@szabinst szabinst left a comment

Choose a reason for hiding this comment

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

I had a few remarks about the code and I'd love to know if you could make the animation a bit smoother.

Now the text input rectangle is too visible compared to the background.

@reabbotted
Copy link
Author

@szabinst How about this? Instead of an opacity fade, I animate the height. I think it looks nice.

Simulator.Screen.Recording.-.iPhone.16.Pro.Max.-.2025-02-12.at.10.50.06.mp4

@reabbotted reabbotted requested a review from szabinst February 12, 2025 17:51
Copy link
Contributor

@Ahmed-Naguib93 Ahmed-Naguib93 left a comment

Choose a reason for hiding this comment

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

Thanks for the great work! It's working fine, but I hope that in the next big task, you try to divide it into smaller tasks and smaller PRs.
1-Please add your new Storybook component, like TextField, to the Design System tab as a reference for us in the future, as we have done.
2- In the Advanced settings, when I open the time zone dropdown list, I can't dismiss it until I select a time. The keyboard also stays open. Please make it so that tapping outside closes both the dropdown and the keyboard.
3- Now in the advanced setting i got an error at log but can't see any alert for the error, please check the error handling
{"status":"unauthorized","errors":[{"message":"user not authorised to perform that action"}]} keyNotFound(CodingKeys(stringValue: "id", intValue: nil), Swift.DecodingError.Context(codingPath: [], debugDescription: "No value associated with key CodingKeys(stringValue: "id", intValue: nil) ("id").", underlyingError: nil))

4- In the profile too, got an error at log can't see any alert for error & check to dismiss the keyboard when tap outside.

{"name":"Ahmed Naguib","id":"1306","created_at":"2024-08-09T02:11:51+02:00","sortable_name":"Naguib, Ahmed","short_name":"Ahmed Naguib ","sis_user_id":"student","integration_id":"password","sis_import_id":null,"login_id":"Ahmed.Naguib","avatar_url":"https://horizon.cd.instructure.com/images/messages/avatar-50.png","avatar_state":"none","email":"[email protected]","locale":null,"time_zone":"Rome"} typeMismatch(Swift.Dictionary<Swift.String, Any>, Swift.DecodingError.Context(codingPath: [CodingKeys(stringValue: "time_zone", intValue: nil)], debugDescription: "Expected to decode Dictionary<String, Any> but found a string instead.", underlyingError: nil))


init(api: API = AppEnvironment.shared.api) {
self.api = api
}
Copy link
Contributor

Choose a reason for hiding this comment

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

The API not used, please remove it

szabinst
szabinst previously approved these changes Feb 13, 2025
Copy link
Contributor

@szabinst szabinst left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. The animation is much better!

Copy link
Contributor

@Ahmed-Naguib93 Ahmed-Naguib93 left a comment

Choose a reason for hiding this comment

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

Thanks for your changes

@szabinst szabinst merged commit 2bcb573 into feature/horizon Feb 14, 2025
2 of 4 checks passed
@szabinst szabinst deleted the CLX-432-i-os-profile-settings branch February 14, 2025 09:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants