Skip to content

FAB iOS Fix and E2E Link Fix#2366

Merged
ayush547 merged 4 commits into
microsoft:mainfrom
ayush547:FAB-Fix
Nov 29, 2022
Merged

FAB iOS Fix and E2E Link Fix#2366
ayush547 merged 4 commits into
microsoft:mainfrom
ayush547:FAB-Fix

Conversation

@ayush547
Copy link
Copy Markdown
Contributor

@ayush547 ayush547 commented Nov 29, 2022

Platforms Impacted

  • iOS
  • macOS
  • win32 (Office)
  • windows
  • android

Description of changes

FAB for iOS was not rendering properly after the tokenization PR #2337

This was due to lack of proper token values being present after FAB completion. As a temporary fix until FAB iOS is tokenized, have changed the structure to a single tokens file for both iOS and Android.

Repo Links tests was failing after E2E shifted to its own package PR #2345
Link - https://github.com/microsoft/fluentui-react-native/tree/main/apps/fluent-tester/src/E2E#authoring-e2e-test shifted to https://github.com/microsoft/fluentui-react-native/tree/main/apps/E2E#authoring-e2e-test

Verification

Before After
MicrosoftTeams-image (9) MicrosoftTeams-image (11)

Pull request checklist

This PR has considered (when applicable):

  • Automated Tests
  • Documentation and examples
  • Keyboard Accessibility
  • Voiceover
  • Internationalization and Right-to-left Layouts

@ayush547 ayush547 requested a review from a team as a code owner November 29, 2022 10:33
@ayush547 ayush547 changed the title [FAB] iOS Fix FAB iOS Fix and E2E Link Fix Nov 29, 2022
@ayush547 ayush547 merged commit eecdb6e into microsoft:main Nov 29, 2022
@ayush547 ayush547 deleted the FAB-Fix branch November 29, 2022 17:33
@lyzhan7
Copy link
Copy Markdown
Contributor

lyzhan7 commented Dec 1, 2022

Just pulled in the latest changes and confirmed that the iOS FAB is no longer broken.
I had a screen recording of the iOS button test page from about a week ago and noticed a couple differences from how it looks now:

  • is the FAB supposed to have some kind of outline/background? In the 'after' screenshot you posted above it looks like a "subtle" button, when before it looked more like a circular primary button
  • the default and primary buttons on iOS look the same now. I think on Android that was an intentional decision, just wanted to double check that that was an intentional change for iOS as well?

For reference here's a screen recording of the iOS Button test page from a week ago vs. just now

ios_button_nov22.mp4
ios_button_nov30.mp4

@ayush547
Copy link
Copy Markdown
Contributor Author

ayush547 commented Dec 1, 2022

@lyzhan7 FAB on Android adopts 'primary' (or 'ascent') as its appearance by default and looks as follows -
image
Which is similar to what FAB on iOS by default!

(In fact they have the exact same color token for background color 'brandBackground'!) These color tokens are not present for iOS at the moment and that is why FAB appears like a subtle button without a background. This will automatically be fixed when tokenization is done for iOS.

@Saadnajmi
Copy link
Copy Markdown
Collaborator

@lyzhan7 FAB on Android adopts 'primary' (or 'ascent') as its appearance by default and looks as follows -
image
Which is similar to what FAB on iOS by default!

(In fact they have the exact same color token for background color 'brandBackground'!) These color tokens are not present for iOS at the moment and that is why FAB appears like a subtle button without a background. This will automatically be fixed when tokenization is done for iOS.

I thought this revert brought FAB on iOS back to what it looked like before 😅. I don't think it's a sufficient fix for us to say FAB will be fixed when it's tokenized. I'm sorry if that was unclear earlier

@lyzhan7
Copy link
Copy Markdown
Contributor

lyzhan7 commented Dec 1, 2022

Also just wanted to note another issue with using the Android tokens for iOS - the iOS shadow won't work as expected. iOS doesn't use elevation like android does. The 'shadowToken: t.shadows.shadow8" line is needed to get the iOS shadows on FAB, and it's been deleted now. I think for this we need to have a broader discussion on what to do - I think it might be a good time to invest making the Shadow component work for Android as well.

@ayush547 ayush547 restored the FAB-Fix branch December 2, 2022 06:28
@ayush547
Copy link
Copy Markdown
Contributor Author

ayush547 commented Dec 2, 2022

Reopening PR to revert FAB on iOS.

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.

4 participants