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

Added large custom view size to MSFTableViewCell #2114

Closed

Conversation

nabil-k
Copy link

@nabil-k nabil-k commented Jan 13, 2025

Platforms Impacted

  • iOS
  • visionOS
  • macOS

Description of changes

  • Added a large custom view size option MSFTableViewCell to accommodate larger custom views.
  • TableViewCell.swift
    • Forcing two line layout when using the new large size if the layout type was trying to be one line.
  • Added the large custom view size example to the demo app

Binary change

N/A

Verification

When using a fluent avatar with size 40 and then setting an activity, for example:

let avatar = MSFAvatar(style: .default, size: .size40)
avatar.state.activityStyle = .circle
avatar.state.activityImage = UIImage(systemName: "star")
avatar.state.image = UIImage(systemName: "person")

The avatar view will actually go from having 40x40 dimensions, to having 46x46 dimensions. When using the fluent table view cell which can only accommodate 40x40, this causes the unread status to make contact with the custom view. However, when using the new large custom view, that is no longer the case.

Before (Using medium custom view size) After (using new large custom view size)
image image
Before (Not forcing two line layout for large custom view size) After (forcing two line layout for large custom view size)
image image

Large custom view size added to testapp:
image

Pull request checklist

This PR has considered:

  • Light and Dark appearances
  • iOS supported versions (all major versions greater than or equal current target deployment version)
  • VoiceOver and Keyboard Accessibility
  • Internationalization and Right to Left layouts
  • Different resolutions (1x, 2x, 3x)
  • Size classes and window sizes (iPhone vs iPad, notched devices, multitasking, different window sizes, etc)
  • iPad Pointer interaction
  • SwiftUI consumption (validation or new demo scenarios needed)
  • Objective-C exposure (provide it only if needed)
Microsoft Reviewers: Open in CodeFlow

@nabil-k nabil-k requested a review from a team as a code owner January 13, 2025 19:21
@joannaquu
Copy link
Contributor

have we run this pass design yet?

@nabil-k
Copy link
Author

nabil-k commented Jan 13, 2025

Haven't followed up with any designers about this regarding fluent specifically.

@anandrajeswaran
Copy link
Contributor

Is the issue really that we need a new size? Or is it just a layout (implementation) issue at an existing size?

@nabil-k
Copy link
Author

nabil-k commented Jan 13, 2025

Followed up with @joannaquu offline and reaching out to designers. Will provide an update when I hear back.

@anandrajeswaran I think the issue is that we'd need a new size. From that I've seen in a design the avatars with activities are supposed to be 46x46. But I'll pass this question along with design and see what they say about if the adjustment that needs to be made is related to the table view cell or the avatar.

@nabil-k
Copy link
Author

nabil-k commented Jan 14, 2025

Spoke with design and they were okay with this change. They didn't have any opinions though on if the change should be made to the avatar v.s. the table cell.

@anandrajeswaran
Copy link
Contributor

Followed up with @joannaquu offline and reaching out to designers. Will provide an update when I hear back.

@anandrajeswaran I think the issue is that we'd need a new size. From that I've seen in a design the avatars with activities are supposed to be 46x46. But I'll pass this question along with design and see what they say about if the adjustment that needs to be made is related to the table view cell or the avatar.

Can you add an example in the Demo app? May be interesting to show how this looks relative to other similarly sized cells and what impact that has on consuming code

@nabil-k
Copy link
Author

nabil-k commented Jan 14, 2025

@microsoft-github-policy-service agree company="Microsoft"

@joannaquu
Copy link
Contributor

sorry, just thought of one more thing to check. MSFTableViewCellCustomViewSize should be used in ListItem, as well PersonaCell, PopupMenuCell. can you double check that this change does not affect them?

@nabil-k
Copy link
Author

nabil-k commented Jan 16, 2025

Checked with those controls and didn't see they'd be affected. But I'm going to close this PR since I was able to get the same results by just overriding the tokens for the custom view. So this PRs changes don't need to be checked in.

@nabil-k nabil-k closed this Jan 16, 2025
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.

3 participants