[BUG] Bug fixes on the stories sites bottomsheet

manav2modi commented 3 weeks ago

Description

Please provide a summary of the pull request and the issue it fixes. Please add necessary details, context, dependencies, explanation of when review is needed (see next section), etc.

Fixes #4454

Review Time Estimate

Please give your idea of how soon this pull request needs to be reviewed by selecting one of the options below. This can be based on the criticality of the issue at hand and/or other relevant factors.

[ ] Immediately
[ ] Within a week
[ ] When possible

Type of changes

Please select a relevant option:

[ ] 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 not work as expected).
[ ] Other (any another change that does not fall in one of the above categories.)

Checklist:

Please select all applicable options:

[ ] I have signed the Rokwire Contributor License Agreement (CLA). (Any contributor who is not an employee of the University of Illinois whose official duties include contributing to the Rokwire software, or who is not paid by the Rokwire project, needs to sign the CLA before their contribution can be accepted.)
[ ] I have updated the CHANGELOG.
[ ] I have read the Contributor Guidelines.
[ ] I have performed a self-review of my own code.
[ ] I have commented my code, particularly in hard-to-understand areas.
[ ] My change requires updating the documentation.
[ ] I have made necessary changes to the documentation.
[ ] I have added tests related to my changes.
[ ] My changes generate no new warnings.
[ ] New and existing unit tests pass locally with my changes.
[ ] Any dependent changes have been merged and published in downstream modules.

manav2modi commented 2 weeks ago

Hey @mihail-varbanov. Thanks for the review:

The reason I handled TrackingAuthorizationStatus that way was because if the user wants to not be tracked even after the apple popup they should still be able to open links which they currently are not able to. The way I have set it up opens it on the inapp browser for the user.

1 and 2 have been fixed.

Would you be able to provide some more details on 3 and 4?

Thanks

mihail-varbanov commented 2 weeks ago

Thanks @manav2modi,

Your explanation makes sense. BTW, there is no clear standard when to use WebPanel and when to use an external browser for displaying urls. When we started we used mainly WebPanel but later due to various reasons (incl. iOS Tracking Authorization) we tend to prefer external browsers. What I am trying to say is that if this is not explicitly required by the design, you can always launch the URL in an external browser. In my opinion this would bring and more consistent behavior.

Regarding 3 & 4, I decided that I would rather fix them than pass the ball to each other a few more times. This commit fixes point 3 and this point 4. Please pay attention to them and try to take them into account in your future software development for Illinois app.

rokwire / illinois-app