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

add logic to show introduction paragraph is presented differently in the editing view #4790

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

rajparad
Copy link

@rajparad rajparad commented Mar 24, 2024

Phabricator: https://phabricator.wikimedia.org/T212318

Notes

  • This PR covers logic to show alert when clicking on edit introduction. Alert has been shown only once, when clicked "Yes".
  • This is my first PR to WIKI. Need suggestions to confirm the localize string implementation is correct or not. also ticket only mentions to have something so the user knows the order is different. based on that I have used alert to show only once.

Test Steps

  1. Open any article.
  2. Click on pencil in right.
  3. Click on "Edit introduction"
  4. the new alert will appear.
  5. Click "Yes" to proceed, click "cancel" and the repeat the flow again to show Alert.
  6. If clicked "Yes" and want to see Alert again, uninstall app and re-install again.

Screenshots/Videos

  • Eye (Alexander McQueen
  • Eye (Alexander McQueen
  • The ordering of elements will

@rajparad rajparad marked this pull request as ready for review March 24, 2024 16:41
@rajparad rajparad marked this pull request as draft March 24, 2024 16:42
@tonisevener
Copy link
Collaborator

Hi @rajparad! Apologies for the review delay on this one. This all looks good from a code perspective - nice work! I will ask our designers to review further in case they have any additional behavior or copy tweaks before merging. Thanks!

@rajparad
Copy link
Author

Hi @rajparad! Apologies for the review delay on this one. This all looks good from a code perspective - nice work! I will ask our designers to review further in case they have any additional behavior or copy tweaks before merging. Thanks!

No worries, Thanks for the reply!! let me know if there is any feedback from designers :)

@rajparad
Copy link
Author

Hello @tonisevener, There was a feedback from design to show alert. there are 2 ways.

Let me know what is your preference from code perspective. I can update accordingly :).

Design feedback is here: https://phabricator.wikimedia.org/T212318

@tonisevener
Copy link
Collaborator

Hello @tonisevener, There was a feedback from design to show alert. there are 2 ways.

Let me know what is your preference from code perspective. I can update accordingly :).

Design feedback is here: https://phabricator.wikimedia.org/T212318

Hi @rajparad - apologies for the delay. I think we can get pretty close to option #1. You'll want to put in in the correct places in PageEditorViewController. We have a toast component that can be triggered like this:

WMFAlertManager.sharedInstance.showWarningAlert("Text here", sticky: false, dismissPreviousAlerts: true)

The design won't match up exactly, but that is okay.

screenshot

You can trigger within loadContent once all the content is loaded in this method, if no issues have cropped up (i.e. no blocked error, no edit notice is displayed, etc.).

If an edit notice modal is automatically presented, you also want to try triggering it after that edit notice modal is dismissed. Usually the featured article of the day has an edit notice, if you want to test that and see how it behaves.

Hope this helps. Let us know if you have any other questions, thanks!

# Conflicts:
#	Wikipedia/iOS Native Localizations/en.lproj/Localizable.strings
@rajparad
Copy link
Author

rajparad commented May 29, 2024

Hello @tonisevener, Thanks for the feedback. I have updated the code to show warning. For now it's only show one time for a user.

image

Here is the link: https://github.com/wikimedia/wikipedia-ios/pull/4790/files

Only thing I am curious is it's now only work with English. I am not how to work with localization in project. Let me know if it's fine or not.

@rajparad rajparad marked this pull request as ready for review May 29, 2024 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants