Skip to content

Conversation

lyzhan7
Copy link
Contributor

@lyzhan7 lyzhan7 commented Jun 20, 2024

Platforms Impacted

  • iOS
  • visionOS
  • macOS

Description of changes

Continuation of #1939

  • There was a suggestion in that PR to create a separate subspec for both TVC/ListItem to depend on rather than having ListItem depend on all of TVC - followed up on that, and also thought it would be cleaner for the shared tokens/enums that both TVC/ListItem use to be in its own folder, but happy to hear thoughts/go back to only the podspec changes
  • ListItem depended on TVC's tokens, as well as several enums. There were a couple enums used that weren't defined in the TableViewCellTokenSet file - I saw there were a couple other enums defined in TableViewCellTokenSet, so I just moved all the shared ones into that file - but maybe would make more sense to put all the enums in its own file

Binary change

Not adding/removing any source code, so I don't think this PR should impact binary size - but if there's a reason to run this let me know
(how is our binary size impacted -- see https://github.com/microsoft/fluentui-apple/wiki/Size-Comparison)

Verification

pod lib lint passes. Unfortunately each run of this seemed like it took about 20 minutes. If there's a faster way to validate podspec changes in fluentui-apple let me know!

Microsoft Reviewers: Open in CodeFlow

@lyzhan7 lyzhan7 marked this pull request as ready for review June 21, 2024 01:53
@lyzhan7 lyzhan7 requested review from a team as code owners June 21, 2024 01:53
Copy link
Collaborator

@mischreiber mischreiber left a comment

Choose a reason for hiding this comment

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

Looks good overall! I'm debating about the name TableViewListShared, but I also don't have a better idea.

@lyzhan7 lyzhan7 merged commit 2d1071a into microsoft:main Jun 21, 2024
@lyzhan7 lyzhan7 deleted the user/lyzhan/listitem-podspec branch June 22, 2024 01:59
@mischreiber mischreiber mentioned this pull request Jun 24, 2024
3 tasks
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.

5 participants