Skip to content

Add header view to ActionSheetViewController - #18415

Merged
wargcm merged 2 commits into
trunkfrom
feature/18402-bottom-sheet-header-view
Apr 25, 2022
Merged

Add header view to ActionSheetViewController#18415
wargcm merged 2 commits into
trunkfrom
feature/18402-bottom-sheet-header-view

Conversation

@wargcm

@wargcm wargcm commented Apr 21, 2022

Copy link
Copy Markdown
Contributor

Ref: #18402

Description

Allows for a header view to be added to the action sheet view controller. This will be used in a future PR for the blogging prompts header view.

Testing

To test:

  • Sign into the app and navigate to the 'My Site' tab
  • Tap on the floating create new action button
  • Verify it functions as it previously did

Regression Notes

  1. Potential unintended areas of impact
    N/A

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    N/A

  3. What automated tests I added (or what prevented me from doing so)
    N/A

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@wargcm wargcm added this to the 19.8 milestone Apr 21, 2022
@wargcm
wargcm requested a review from ScoutHarris April 21, 2022 18:03
@wargcm wargcm self-assigned this Apr 21, 2022
@wargcm
wargcm force-pushed the feature/18402-bottom-sheet-header-view branch from 2318b94 to 4fe7e52 Compare April 21, 2022 18:20
@wpmobilebot

wpmobilebot commented Apr 21, 2022

Copy link
Copy Markdown
Contributor
You can test the changes in Jetpack from this Pull Request by:
  • Clicking here or scanning the QR code below
  • Then installing the build number pr18415-2ecd804 from App Center on your iPhone

The .ipa file can also be downloaded directly here.
If you need access to App Center, please ask a maintainer to add you.

@wpmobilebot

wpmobilebot commented Apr 21, 2022

Copy link
Copy Markdown
Contributor
You can test the changes in WordPress from this Pull Request by:
  • Clicking here or scanning the QR code below
  • Then installing the build number pr18415-2ecd804 from App Center on your iPhone

The .ipa file can also be downloaded directly here.
If you need access to App Center, please ask a maintainer to add you.

@ScoutHarris ScoutHarris left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Hey Chris. I had one minor comment, but otherwise looks good!

:shipit:

let headerView: UIView?
let buttons: [ActionSheetButton]
let headerTitle: String
let buttonHeaderTitle: String

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'm a bit confused about this name change. It looks like it's now a button title, but it's still the same header title. What it meant to clarify its use? I'm not sure it did. Maybe we can change it back?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I can change it back. I just thought that it'd be confusing to have both the header view and something called a header title. Adding button was just to try and clarify it is the title for the buttons.

@wargcm
wargcm merged commit 3869aa9 into trunk Apr 25, 2022
@wargcm
wargcm deleted the feature/18402-bottom-sheet-header-view branch April 25, 2022 14:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants