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

[feature/new-issue-view] New Issue view / presentation #874

Merged
merged 8 commits into from
Feb 1, 2021

Conversation

felix-schwarz
Copy link
Contributor

@felix-schwarz felix-schwarz commented Jan 23, 2021

Description

As fixing an iPad layout issue in the old issues view proved too cumbersome, I've replaced the entire implementation with a new issue view, based on code already there and in use for cards and tables.

The result provides a more modern look, is consistent with the rest of the app and shrinks the code base by ~ 220 lines of code.

Screenshots (if appropriate):

Previously Certificate warning
Simulator Screen Shot - iPhone 8 - 2021-01-23 at 13 13 48
Previously Error
Simulator Screen Shot - iPhone 8 - 2021-01-23 at 13 16 45

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

QA

Checklist: #874 (comment)

Bugs & improvements:

- ConnectionIssueViewController:
	- compare against certificate stored in the bookmark where available
	- remove text-based rendering via CertificateViewController (and remove the class), replace it with the standard certificate view controller
- adaptions to new ThemeCertificateViewController initialization where needed
- update SDK
	- remove IssuesViewController, ConnectionIssueViewController, IssuesPresentationAnimator and IssuesDismissalAnimator
	- create modern replacement leveraging existing code: IssuesCardViewController
	- migrate code from ConnectionIssueViewController to IssuesCardViewController
	- add new helper function to compare DisplayIssues issue levels without having to use .rawValue
- MoreViewController
	- becomes FrameViewController
	- adds the ability to also add a footer view
	- cleanup: removal of unused properties and initializers
- StaticTableViewRow: add .messageStyle support
- ThemeTableViewCell: add .messageStyle support and add additional CellStyler block API based on also new CellStyleSet
- AlertView:
	- add support for .accessibilityIdentifier
	- add option for custom .contentPadding
	- add button-only mode
- CardPresentationController:
	- ensure .dismissable is actually respected
	- fix premature release of CardTransitionDelegate that could lead to normal presentation rather than as a card
@felix-schwarz felix-schwarz added this to the 11.5.0-Current milestone Jan 23, 2021
@CLAassistant
Copy link

CLAassistant commented Jan 23, 2021

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ hosy
❌ felix-schwarz
You have signed the CLA already but the status is still pending? Let us recheck it.

@felix-schwarz felix-schwarz changed the base branch from feature/tls-diff to milestone/11.5 January 25, 2021 09:45
@jesmrec
Copy link
Contributor

jesmrec commented Jan 26, 2021

QA checks

iPhone

iPad

@jesmrec
Copy link
Contributor

jesmrec commented Jan 26, 2021

(1)

The Review Connection message when http connection is stablished in landscape

Screen Shot 2021-01-26 at 12 05 00

text does not fit correctly to the background yellow chart.

If portrait mode is set again after landscape:

Screen Shot 2021-01-26 at 12 05 05

iPhoneXR iOS14
iPadAir iOS13

- FrameViewController: include table background color pin theming support
- ThemeTableViewCell:
	- support recreation of labels and custom layout of these labels
	- add primaryTextLabel and primaryDetailTextLabel accessors to access text labels irrespective of whether they are system-provided or recreated
- StaticTableViewRow
	- add support for recreated labels and their custom layout
- IssuesCardViewController
	- replace usage of constraints in CardCellBackgroundView with auto resized views (fixes auto layout constraint warnings)
	- switch to using primaryTextLabel and primaryDetailTextLabel and a custom recreatedLabelLayout
@felix-schwarz
Copy link
Contributor Author

@jesmrec Thanks. I could reproduce this only when rotating the device. It boiled down to iOS not respecting the provided margins after rotating the table cell view. Fixed as of 83eeab1.

@jesmrec
Copy link
Contributor

jesmrec commented Feb 1, 2021

Totally fixed. Approved on my side

@jesmrec jesmrec added the Approved by QA Approved by QA label Feb 1, 2021
# Please enter a commit message to explain why this merge is necessary,
# especially if it merges an updated upstream into a topic branch.
#
# Lines starting with '#' will be ignored, and an empty message aborts
# the commit.
@hosy hosy merged commit 7a7e825 into milestone/11.5 Feb 1, 2021
@delete-merged-branch delete-merged-branch bot deleted the feature/new-issue-view branch February 1, 2021 10:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Approved by QA Approved by QA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants