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.

Sign up for GitHub

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

Jump to bottom

Get theme from parent to update ShyHeaderController colors #1564

Merged
merged 2 commits into from
Feb 13, 2023
Merged

Get theme from parent to update ShyHeaderController colors #1564

merged 2 commits into from
Feb 13, 2023

Conversation

laminesm
Copy link
Contributor

@laminesm laminesm commented Feb 10, 2023 •
edited
Loading

Platforms Impacted

  • iOS
  • macOS

Description of changes

There's a bug where the ShyHeaderController wasn't using the correct theme to update colors. This PR fixes the bug by passing down a weak reference of the view context to make sure ShyHeaderController uses the correct theme to update its colors. Also, viewDidAppear has been removed and the color updates will be called in viewWillAppear.
This will be a short term fix, as we will be looking into ways to delegate ShyHeaderController color updates to NavigationController in the longer term.
This PR also fixes a bug introduced by #1050 where the SearchBar doesn't use the right branding colors. The brandContent style for the SearchBar has been removed to partially revert the change and fix the bug.

Binary change:

File Before After Delta
libFluentUI.a 29,075,768 bytes 29,085,640 bytes 9,872 bytes

Verification

Before After
before after
before_blue after_blue
before_green after_green
before_light after_light
before_dark after_dark

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

@laminesm laminesm requested a review from a team as a code owner February 10, 2023 23:10
@laminesm laminesm merged commit a7a6e2a into microsoft:main Feb 13, 2023
@laminesm laminesm deleted the laminemale/fix-shy-header branch February 13, 2023 18:48
@harrieshin harrieshin mentioned this pull request Feb 14, 2023
Merged
11 tasks
joannaquu added a commit that referenced this pull request Mar 3, 2023
@joannaquu @laminesm
@lukas-cap @harrieshin @huwilkes @github-actions @markavitale @mischreiber @sophialee0416 @jeaniehuynh @nodes11 @alexanderboswell
[fluent2-tokens] Merging main -> fluent2-tokens (#1622)
4edfa09 
* Fix PillButton colors (#1560)

* Remove duplicated color assignments

* Fix pressed colors

* Fix rest colors

* Fix active colors

* Fix disabled colors

* Fix ColoredPillBackgroundView colors

* Fix linting error

* Fix bottom sheet demo safe area issue (#1565)

* didmovetowindow to willMove:toWindow (#1563)

we shouldn't use didmovetowindow for updating colors. we should use willmove:towindow so we can handle animations better

* Get theme from parent to update ShyHeaderController colors (#1564)

* Get theme from parent

* Fix searchbar colors

* Fix bottom sheet shadows (#1568)

* rename buttonSize to ButtonSizeCategory (#1567)

Button's variable size seems confusing to the CGRect's size. Change the name to ButtonSizeCategory.
Add another demo controller case where we host Fluent button in toolbar.

* make sure headerfooterview isn't clear background and actually listen to alias token (#1570)

If the tableviewStyle is .plain, because the headerfooterview by default is .clear color, when the cell scrolls, it draws in the background. Even though TableViewHeaderFooterView isn't fluent2 tokenize yet, lets use the alias token to update the background color, if the client configures what TableViewStyle this headerfooter view is hosted in.

* Add TextField a11y label (#1569)

* Latest localized resource files from Touchdown Build (#1572)

Co-authored-by: huwilkes <huwilkes@users.noreply.github.com>

* the last cell of the section in insetgroup style shouldn't have a bottom separator (#1571)

Last cell of the insetGroup section shouldn't have a separator

* Update maxFontSize to 12 in TabBarItemView (#1574)

* bug fixes on colors around drawercontroller and bottomsheet (#1573)

*BottomSheet in darkmode uses darkelevated. so using just alias background 2 isn't goodenough
*Badgefield and people picker rest state on darkmode seems to be not correct
*bottomsheet demo color on darkmode should be more consistent
*light mode default style badgeview(branding color) should have its own disabled color that is branding color
*bottomcommandingbar headerfooterview color in darkmode didn't seem right.

* update to 0.13.0 (#1575)

* Latest localized resource files from Touchdown Build (#1576)

Co-authored-by: markavitale <markavitale@users.noreply.github.com>

* communicationTextColor should use global tokens (#1577)

* Latest localized resource files from Touchdown Build (#1580)

Co-authored-by: markavitale <markavitale@users.noreply.github.com>

* Fixing initial theme setting for `willMove(toWindow:)` (#1581)

* Update all tokenized components

* Use EmptyTokenSet to fix non-tokenized controls

* Fixing whitespace (spaces, not tabs)

* Documentation

* Lighten all dark segmented controls (#1585)

* Lighten all dark segmented controls

* Fixed whitespace

* Change colors to be more like real world

* Latest localized resource files from Touchdown Build (#1584)

Co-authored-by: markavitale <markavitale@users.noreply.github.com>

* Updating swift version to 5.7.1 (#1588)

* Updating swift version to 5.7.1

* Updating readme

* [FHL] Add script to generate lib binary change breakdown (#1589)

* Add script to generate lib binary change breakdown

* Change column titles to upper case

* reorganize file so input validation actually works

* Change to sort without abs, add emoji

* SizeDiff -> SizePair

* Lots of changes, sorry

* Rename to GenerateBinaryDiffTable

* Swap danger/severe

* Right align number columns, make detailed table collapsible

* Add brand style to SearchBar demo (#1591)

* Add brand style to searchbar demo

* Add custom stackview to change the background

* Update PR Template (#1593)

* Add XCUI tests for bottomsheet (#1594)

* Add XCUI tests for bottomsheet

* Check sheet exists before scrolling

* Fix BadgeLabel to use `tokenSet.fluentTheme` (#1595)

* Fix BadgeLabel to use `tokenSet.fluentTheme`

* Minor cleanup of code for readability

* Make BadgeView's accessibilityLabel customizable (#1592)

* make badge view accessibility label settable

* change to pull from dataSource's accessibilityLabel

* remove set

* Adding an explicit `import Combine` for Xcode 14.3 (#1599)

* Add the TextField (#1540)

* Add naive textfield

* Add demo page

* Add tokens

* Use tokens

* Update to fluent 2 colors, mostly

I decided to just paste in the values of the alias tokens in the designs, since those tokens are in fluent2-colors, not main. Except for the brand colors, switched those to just rest, so that it will respond to brand overrides, still.

* Add the correct icon

* Remove focused state

* Add placeholderColor token

* Move attributedPlaceholder

* Make the textfield final and more objc friendly

* Simplify state, again

Turns out that .placeholder and .filled were always returning the same values, so they're likely the same state. Which is when the textfield doesn't actively have focus.

* Add error and initial text validation hooks

* Update demo

* Add leading image container

* Expose text

* Add hook for didBeginEditing

* Adjust demo to show more variety

* Use our label

* Adjust hooks

* Change label of the second demo textfield

* un-finalize the text input error

* Make error open

* Change validateInputText to onEditingChanged

* Add accessibility label for clear button

* Add comments

* Use background color token

* Split classes into new files

* Fix demo lint errors

* Add TextField to podspec

* reorder textfield files

* Update tokens

* Fix usage of other Fluent controls

* Scaling fonts for input/placeholder

* Fix line break of top label

* Add objc demo

* Add override tokens in the Swift demo

* Add comment explaining the internal text field

* Update icon size token

* Change name of Text Field demo

* Fix lint errors

* Add preserve_paths to podspec

* Switch to directional insets

* Switch to static let

* Actually switch to lets

* Stack cleanup

* Comment cleanup

* Change error description to let

* Change to willMove(toWindow:)

* Clean up old accessibility label

* Add launch test

* Rename labelText and text

* Simplify logic lets

* Fix lint error

* Update comments

* Change dismiss circle to svg

* Sort imports

* Change to init(frame: )

* Make tokenSet's state private

* Remove custom init from objc demo

* PillButton attributedTitle is showing the wrong font size (#1597)

* PillButton attributedTitle is showing the wrong font size

Use of `.fluent(fontInfo:)` inside PillButton was resolving to `Font.fluent(fontInfo:)` and not `UIFont.fluent(fontInfo:)`. This was causing the wrong font size to be used for the PillButton.

* PillButton text should not scale with DynamicType. (#1601)

Set the `shouldScale` parameter to `false`.

* Tokenize ShyHeaderView (#1603)

* Tokenize ShyHeader

* Fix lint issue

* Make parentController weak

* Tokenize Label (#1602)

* Tokenize Label

* Tokenize font

* Update font on willMove(ToWindow:)

* consider padding in scrollable segmented control(#1609)

When the segmented control only have 4 items that kind of fits into the scrollview content, the graident wouldn't sometimes show. When we calculate the offset of when to show the gradient, make sure we account for the inset of the stackview inside the scroll view

* Deprecate file accessory view (#1612)

deprecate TableViewCellFileAccessoryView

* fix

---------

Co-authored-by: Lamine Male <106181067+laminesm@users.noreply.github.com>
Co-authored-by: Lukas Capkovic <3610850+lcapkovic@users.noreply.github.com>
Co-authored-by: Harrie Shin <hyshin@microsoft.com>
Co-authored-by: huwilkes <67026548+huwilkes@users.noreply.github.com>
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: huwilkes <huwilkes@users.noreply.github.com>
Co-authored-by: markavitale <markavitale@users.noreply.github.com>
Co-authored-by: Mike Schreiber <mischreiber@microsoft.com>
Co-authored-by: Sophia Lee <sophia.lee0416@gmail.com>
Co-authored-by: Jeanie Huynh <31874971+jeaniehuynh@users.noreply.github.com>
Co-authored-by: Des Marks <johnmarks@microsoft.com>
Co-authored-by: Alexander Boswell <alboswel@microsoft.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@edjamesmsft edjamesmsft edjamesmsft approved these changes

@harrieshin harrieshin harrieshin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@laminesm @harrieshin @edjamesmsft