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

WidgetsSDK Release 2.2.0 #810

Merged
merged 40 commits into from
Oct 20, 2023
Merged

WidgetsSDK Release 2.2.0 #810

merged 40 commits into from
Oct 20, 2023

Conversation

EgorovEI
Copy link
Contributor

@EgorovEI EgorovEI commented Oct 20, 2023

What was solved?

Release notes:

  • Feature
  • Ignore
  • Release notes (Is it clear from the description here?)
  • Migration guide (If changes are needed for integrator already using the SDK - what needs to be communicated? Add underneath please)

Additional info:

  • Tests fixed, added? Unit, acceptance, snapshots?
  • Logging necessary for future troubleshooting of customer issues added?

Screenshots:

Egor Egorov and others added 30 commits October 19, 2023 17:49
Before attachment button was hidden when Response/Custom card was received. Now the button visibility behaviour is aligned with entry field blocking behaviour. If sending media is allowed in site setting, now the button is hidden only during enqueueing and transferring state.

MOB-2636
This PR adds a capability to snapshot test screens in landscape, both
with regular and large font.

MOB-2653
In order to keep existing behaviour and avoid message duplication, messages placed in pending section once delivered via socket are discarded.

MOB-2650
This PR is experimental, yet important first step towards transitioning
to SwiftUI. ConfirmationView has been rewritten entirely in SwiftUI. The
primary motivator for this PR was the frustrating nature of UIKit constraints.

Key factors to keep in mind while reviewing:

1. Checkmark image has been made smaller, which was the request from the
design team

2. UIFont does not translate 1:1 to SwiftUI Font, meaning the same font
looks just a little big different now. But dynamic scaling works, and all
our default fonts translate to expected outcome (font size, weight etc.)

3. Some approaches used in view layouts is due to the restrictions of
SwiftUI 1.0 capabilities. Those can and will be upgraded in the future

4. The naming of Views, and Objects, as well as file locations in the
directory are up for discussion. Everyone's input is much appreciated.

5. Prefix SwiftUI is used in lots of places due to a conflict with existing
custom objects Button and Image

6. New HeaderSwiftUI view does not include all the possible configuration
possibilities, as confirmations view's needs are not very demanding. So,
that view can be upgraded in future.

This PR also covers the ticket MOB-2488 which focuses on ADA-compliance
in ConfigurationsView
After migration to SwiftUI, Secure Conversation’s Confirmation header close button is missing 'header_close_button' identifier, which results in acceptance tests failure. Since we are constrained to iOS 13 deployment target, it made sense to add dedicated view modifier to resolve between depreciated and new methods for providing accessibility identifier to avoid deprecation warnings. This modifier will become obsolete once we bump up deployment target to iOS 14.

MOB-2694
This PR adds snapshot tests in landscape mode for regular and large
font types.

MOB-2683
This PR improves the way snapshot tests are executed. By creating an
extension to UIViewController, we can now create snapshot tests with
less code, time, and effort. And, it is much more readable. Since our
usages of snapshot tests are very limited (for now), this approach serves
those purposes very well.

As this proposal is up for debate, I would like to hear everybody's input
on it. Please share what are your thoughts on making this change.

MOB-2696
There were a couple of errors in naming and in the strings, so I updated them.

MOB-2507
The changes in this file were done partially. This is because there are some
strings that are using parameters in the string which are replaces on runtime.
These need to be evaluated to see if a backwards compatibility layer is needed
for them or not, which is out of the scope of this task at the moment.

MOB-2538
A couple of strings have been left untouched because they don't use secure
conversations specific strings. They use chat strings. So these will be done in
a future PR to avoid making the PR too big.

MOB-2538
This time, strings in screen sharing, alert, call, and survey mocks were
changed. Also a string that was previously missing was added.

MOB-2538
Also some strings have been changed as part of several discussions, so they have
been updated here.

MOB-2538
All remaining strings that have a direct translation in the new Localization
enum are now removed. The only strings remaining are the ones that need a
translation layer, because they use variables.

MOB-2538
There was a small amount of strings that I incorrectly identified as needing
additional work to change, but it was just a matter of replacing because they
are all internal and not exposed to the integrators.

MOB-2507
Instead of creating a layer that verifies if a string has a template or not,
the templates (for example, `{operatorImage}`) have been captured in an
internal extension of the `Localization` struct. That way integrators have a
deprecation that they shouldn't use our strings anymore in case they do, but
also the minimum amount of changes is done to the workings on the SDK. So we can
still use templated strings as right now without going and changing a bunch of
SDK functionality.

MOB-2507
The proposal is that the Core SDK will return to us a dictionary with the keys and
values just as they come from the backend. This is also exactly how the strings
are saved in the `Localizable.strings` file, accessed through the `Localization`
enum. When the Core SDK is configured and is able to provide the dictionary, we
save it to a StringProviding struct. When a string is accessed through the
use of `Localization`, then `Localization` knows what is the key of the string,
and also specifies a fallback in case reading from the file fails. So with this
same key, we can go to the StringProviding, consult the dictionary, and if the
string is not there, use the file fallback, and if not use the string fallback.
This means that with this simple PR, once the Core SDK provides the dictionary,
we support custom locales for the whole Widgets SDK with minimal effort. The
alternative is to create a model which we need to keep in sync between Core and
Widgets, which seems like a big mess.

MOB-2282
StringProviding is set when configuration is done. Then this StringProviding will be used to retrieve remote string if exists, otherwise localized value is taken from local file.
In this commit, support for adding a company name from custom locales is added.
However, if the integrator has set this name through the theme, that one is used
instead of the custom locales. Also, in case the custom locale doesn't have a
string for the company name, but the integrator has specified a company name
through the configuration of the SDK, then that is used. Finally, if neither of
the previous possibilies have been used, then the local fallbacks are used.
Together, the first and the last point mean that there are no backwards
compatibility issues, as if the integrator hasn't set any name, still the
local fallback will be displayed, which is "CompanyName". If they have, then
that is used immediately without caring about configuration or custom locales.

MOB-2686
Local key/value pairs were updated to align localization between platforms and remote default locale

MOB-2719
This PR converts ScreenSharingView to SwiftUI

MOB-2720
Global colors were not applied to all views and this PR fixes this issue

MOB-2679
This PR fixes the identifier that lead to acceptance tests to fail
Change accessibility identifier for check messages back to expected name on Confirmation screen.
This commit syncs the leave queue alert to how Android works. Also, the end
engagement alert now uses the `showsPoweredBy` variable. This means that it will
now respect the global setting rather than always setting it as yes. This would
bring issues if a client pays for hiding powered by, as it would be wrongly
shown in that case.

MOB-2742
The `isRemovedOnCompletion` property was missing, which meant that the animation
never ran, and the spinner was always static. Now the animation runs forever
until removed.

MOB-2743
Before, the UiTheme took the highest priority in the algorithm to determine the
company name. Now the remote company name has the highest priority, with the
added constraint that it cannot be empty, as this will be the default value on
the default locale. In case it is empty, the UiTheme will come afterwards, then
the SDK configuration, and finally the local fallback.

MOB-2733
As discussed in retrospective.
Update GliaCoreSDK to 1.1.4 and remove GliaWidgetsXcf.
Egor Egorov and others added 8 commits October 19, 2023 17:50
On SDK configuration, interactor instance is created and has .none state. When Call screen is loaded, its start() method is called which executes some updates based on current interactor.state. When configure method is called before each engagement start (like we currently do in WidgetsSDK Testing app), CallViewModel.start() does nothing, because interactor.state is .none.
But when WidgesSDK is configured once (for example on the app launch), the same interactor instance is used for all subsequent engagements. When you end first engagement (Chat/Audio/Video, probably CV also), interactor.state is .ended. Then if you start new Audio/Video engagement, CallViewModel.start() calls CallViewModel.call.end() method, which breaks something and connecting state becomes infinite.
This commit fixes calling CallViewModel.call.end() on Audio/Video engagement start.
Commit also make two engagement references weak to avoid retaining it when CoreSDK already released it from memory.
The toggle (UISwitch) was added to Main screen, that provides the ability to disable/enable SDK configuration before each engagement.

MOB-2730
There were some tests that still used L10n strings, which are now deprecated.

MOB-2747
Fix hiding of text input for Secure Transcript flow when Secure Message Center is unavailable.

MOB-2734
Default values weren't selected automatically in surveys that showed a single
option. With this change, a new `defaultOption` is added to the survey and used
in case the selected value is not there. Now this works as it should, and how
it works in Android.

The reason why a default option was added and the default value is not just set
as the selected one, is because the question view starts with props, and the
prop is either selected or not. However, the question view wouldn't know if an
option is selected because of user input or because of the default value. If the
view is selected because of user input, then `opt.select(opt)` needs to be
executed only on user interaction, but in case of default value, it needs to be
run automatically as soon as the view is presented. I don't know how to make
this distinction without having the default value separately.

Also, a snapshot test was added to ensure that when there's a default value,
it's selected correctly.

MOB-2747
If the company name is empty in the custom locales set in the web (as is the
default), then the widgets tried to get it, but as it is empty, then the
fallback was the theme and the configuration. If neither of those was able to
provide a non-empty value, then the idea is to use the local fallback. However,
for this I was mistakenly using the value from `Localizable.tr`. The problem is
that this method first attempts to get it again from the string provider. As we
determined in the first step, this is empty. However, `Localizable.tr` doesn't
filter empty values, as all other strings can legitimately be empty. So the
method returns empty string, and this is shown on the widgets.

The solution was to create a new value in the SwiftGen stencil, which is called
`companyNameLocalFallbackOnly`. As the name says, this variable sends the string
provider as nil, thus ignoring the company name's empty value, and using the
value in the `Localizable.strings`, or the hardcoded "Company Name" as fallback.

For now, no other string is subject to this matter where the string can come
from a lot of various sources, so this approach should be sufficient. If at some
point it happens that quite a lot of strings need to be fetched from various
sources, what can be done is that the stencil can be changed so that instead of
generating `static let`s, we generate `static func`s, and then we can easily
send the string provider as needed.

MOB-2768
There is a workflow in ios-bundle repository called "Post Release" which gets
the version and the checksum from the latest version, sends it to an
`update-dependencies` workflow in ios-sdk-widgets, which then executes a
Fastlane lane that uses that info, updates the templates, and generates the new
files. However for some reason, these version and checksum parameters weren't
used in the calling of the Fastlane lane. Now they are called.

Also, now the replacement of the Core SDK version is done first in the template,
then the template is copied to the root folder, and then it is adjusted for the
latest Widgets version. This way, the root podspec and the  template podspec
always have correct Core version. Before, the template was copied first and then
the replacement of the Core SDK was done. This meant the template was always out
of date.

MOB-2770
Egor Egorov and others added 2 commits October 20, 2023 12:39
There were a couple of instances of unowned self and force unwrapping of a
variable that could (and have) crashed the widgets under certain obscure
circumstances.

MOB-2773
@EgorovEI EgorovEI merged commit 09d384b into master Oct 20, 2023
1 check passed
@EgorovEI EgorovEI deleted the release/2.2.0 branch October 20, 2023 10:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

6 participants