duckduckgo / iOS

DuckDuckGo iOS Application
https://itunes.apple.com/us/app/duckduckgo-privacy-browser/id663592361?mt=8
Apache License 2.0
1.82k stars 414 forks source link

implement contextual dialogs #3039

Closed SabrinaTardio closed 2 months ago

SabrinaTardio commented 3 months ago

Task/Issue URL: https://app.asana.com/0/1204186595873227/1207736729379245/f

Description: Implements the Dialog we will use in the contextual onboarding cc. @alessandroboron

Steps to test this PR:

  1. Use the Preview of the following files: ContextualDaxDialog, ContextualOnboardingList, ContextualOnboardingDialogs
  2. the latest represent the top level ones that will be displayed

Definition of Done (Internal Only):

Copy Testing:

Orientation Testing:

Device Testing:

OS Testing:

Theme Testing:


Internal references:

Software Engineering Expectations Technical Design Template

github-actions[bot] commented 3 months ago

:no_entry_sign: The Asana task linked in the PR description is not added to iOS App Board project.

  1. Verify that the correct task is linked in the PR.
    • :warning: Please use the actual implementation task, rather than the Code Review subtask.
  2. Verify that the task is added to iOS App Board project.
  3. When ready, remove the bot: not in app board label to retrigger the check.
github-actions[bot] commented 3 months ago
Warnings
:warning: PR has more than 500 lines of code changing. Consider splitting into smaller PRs if possible.

Generated by :no_entry_sign: dangerJS against d2ca78422e89a166efe7c4f28675c0326e23b10d

alessandroboron commented 3 months ago

As per MM conversation, there’s an issue with the typing animation. The box grows as the typing goes. We can look at adapting AnimatableTypingText with AttributedString. Having said that I think we should finish the functionality first and look at animations later.

SabrinaTardio commented 3 months ago

As per MM conversation, there’s an issue with the typing animation. The box grows as the typing goes. We can look at adapting AnimatableTypingText with AttributedString. Having said that I think we should finish the functionality first and look at animations later.

I should have fixed it! but have not used the existing stuff when can always see later if we want to combine them.

alessandroboron commented 2 months ago

Good job adapting AnimatableTypingText, it works very well! 👏

The views that show a list of suggestions need a VM, but we can add it in another PR when we have the provider ready.

On another note, there’s a conflict with the main feature branch DaxDialogIntroView. I removed this file DaxDialogIntroView.swift, so it shouldn’t be too bad to resolve it.

SabrinaTardio commented 2 months ago

LGTM, very nice. Agree with @alessandroboron we need a view model though, but can follow up with that.

Yes, I agree too there will be a need for a view model but it didn’t make sense to add it now when it does not do anything yet