Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

181937962 share excitement #457

Merged
merged 6 commits into from
May 23, 2022
Merged

Conversation

androidmaven
Copy link
Contributor

@androidmaven androidmaven commented Apr 25, 2022

  • Match pivotal story

Copy link
Contributor

@alexosugo alexosugo left a comment

Choose a reason for hiding this comment

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

The bottomsheet with the gif doesn't display correctly. See below video.

22-04-27-13-14-30.mp4

Screenshot_20220427-131343

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)
isFullScreen = requireArguments().getBoolean(IS_FULLSCREEN)
if (isFullScreen) setFullScreen()
setFullScreen()
Copy link
Contributor

Choose a reason for hiding this comment

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

If transaction details will always be shown as full screen, let's convert this to a proper fragment. It can be simplified to remove unnecessary code.

Copy link
Member

@davkutalek davkutalek left a comment

Choose a reason for hiding this comment

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

This looks fine codewise. You need to get approval from marketing/graphics for something like this - its a pretty big statement. Also I don't really think its worth the size, looks like it would add ~350KB with just the one gif. But ask marketing what they want to show.

@androidmaven
Copy link
Contributor Author

Thanks, reverting the codebase to match actual spec. Will request for design thoughts on GIFs as a follow up item.

@androidmaven androidmaven force-pushed the 181937962-shareExcitement branch from cfa6487 to 94227cb Compare May 11, 2022 14:30
Copy link
Member

@davkutalek davkutalek left a comment

Choose a reason for hiding this comment

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

LGTM

@alexosugo alexosugo changed the base branch from 181584868-newTDdesign to development May 12, 2022 13:37
@androidmaven androidmaven force-pushed the 181937962-shareExcitement branch from 94227cb to 1cc623f Compare May 19, 2022 15:34
@androidmaven androidmaven merged commit 2c4f7ee into development May 23, 2022
@alexosugo alexosugo deleted the 181937962-shareExcitement branch May 24, 2022 16:13
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants