Skip to content

Conversation

@oguzkocer
Copy link
Contributor

This PR updates DialogHolder to use UiString instead of StringRes Int to make it easier and more structured to use either string resources and regular strings easily. It makes it so that each string is known to be either a resource or a regular string, but never both or never neither. Injected UiHelpers is passed in to increase testability of DialogHolder even though it currently doesn't have tests. (maybe an improvement for the future)

To test:

  • Go to Blog Posts
  • Create a draft (local or remote doesn't matter)
  • Tap on the Publish button and verify the dialog shows up as expected (Title, message, button names are still correct)
  • Cancel the dialog - We don't need to test the functionality of the dialog because this PR doesn't change it
  • Tap on the Trash button and verify the dialog shows up as expected (Title, message, button names are still correct)
  • Cancel the dialog - We don't need to test the functionality of the dialog because this PR doesn't change it

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@oguzkocer oguzkocer added this to the 11.7 milestone Jan 23, 2019
@oguzkocer oguzkocer requested a review from mzorz January 23, 2019 21:31
Copy link
Contributor

@mzorz mzorz left a comment

Choose a reason for hiding this comment

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

Tested, works as advertised.
Looked into the code and tried to see if there was anything I could comment and try to improve - I loved the simplicity of it. 🏅
LGTM! :shipit:

@mzorz mzorz merged commit e500f7f into develop Jan 23, 2019
@mzorz mzorz deleted the issue/dialog-holder-ui-string branch January 23, 2019 22:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants