Onboarding Improvements: New Login Epilogue Screen Design #15036
Conversation
This way every parameter ends up being on a separate line, thus making the 'new SitePickerAdapter(...)' constructor read better.
This way every parameter ends up being on a separate line, thus making the 'holder.showSitesHeading(...)' call read better.
This new login epilogue header will be shown when the 'Onboarding Improvements' feature flag gets enabled, else the existing login epilogue header will be shown. PS: The design on this new login epilogue header is not final yet. More refinements are due when the design gets finalized.
With the new login epilogue header the logic of inflating the layout and showing/hiding the site headings needs to be altered. This commit is utilizing the 'Onboarding Improvements' logic to alternate between the new and existing login epilogue header logic.
The design updates is based on final designs as those were provided by the designer.
Those newly extracted method will help with readability and making sure that the logic can be easily updated based on a feature flags being enabled/disabled, especially for the 'footerHandler()' method.
This new login epilogue screen will be shown when the 'Onboarding Improvements' feature flag gets enabled, else the existing login epilogue screen will be shown. PS: This new login epilogue screen will be shown for 2-3 sites or less only. For more than 3 sites, another new login epilogue screen will be shown instead.
This new expanded login epilogue screen will be shown when the 'Onboarding Improvements' feature flag gets enabled and when the number of sites exceeds the expected number, else the existing login epilogue screen will be shown. PS: This new expanded login epilogue screen will be shown for more that 3 sites. For 2-3 sites or less, the non-expanded new login epilogue screen will be shown instead.
This is done since there are actually two dividers, the 'divider' and the 'item_diver' views, which do not appear on the same screen and as such when one is present the other one is not, and vice versa.
This is a new requirement as per the updated design.
This method will be utilized within the 'LoginEpilogueFragment' to help identify whether the new login epilogue sceen UI needs to be shown or the new expanded one instead (for more than 3 sites).
This is done because this 'bottom_shadow' view will not appear on the same screen every time and it will only appear under sertain condition, like with the existing UI or the new expanded UI.
Instead of initializing the 'SitePickerAdapter' during 'setupContent(...)', this is now done during the 'onCreate(...)' phase of this screen's lifecycle. This is due to the fact that this adapter and its publicly made 'getBlogsForCurrentView()' method will be required to be available during 'createMainView(...)' so that the appropriate new login epiloge screen is being picked up (the normal or expanded one).
This commit is utilizing the 'Onboarding Improvements' feature flag to alternate between the new and existing login epilogue screens.
This commit is utilizing the 'EXPANDED_UI_THRESHOLD' logic to alternate between the expanded and normal new login epilogue screens.
ParaskP7
added
[Type] Task
[Status] Needs Design Review
|
You can trigger optional UI/connected tests for these changes by visiting CircleCI here. |
ParaskP7
changed the title
|
You can test the changes on this Pull Request by downloading the APKs: |
There was a problem hiding this comment.
Thanks for sharing @ParaskP7 , its looking great already. Here are a few things I noticed:
- the dividing line is going straight through the 'or' text, rather than breaking either side
- in landscape mode, with only one site, the 'create site' button is not sticky and is off screen for me. I think it should always be sticky in landscape mode for both 1-3 sites and more site versions.
- the buttons are not wired up yet. I couldn't tap a site. 'Create new site' took me to My Site. Assume this is just not done yet.
- Seeing the 'Choose a site to open' text in serif, I'm not sure we've got the balance right with type sizes yet. Let's discuss options together next week.
There was a problem hiding this comment.
Great job @ParaskP7! Changes are coming along really nicely 🙌. Also, appreciate the in-between refactoring you did to make the code more readable and robust.
I only left one suggestion and a warning for you to consider. Hope it helps.
WordPress/src/main/java/org/wordpress/android/ui/accounts/login/LoginEpilogueFragment.java
Show resolved
Hide resolved
|
👋 @osullivanchris ! Thanks for much for this detailed review! 🙏
You are totally right, really good eye, I totally missed that. It is very strange that this is happening since I copy-pasted the component from the
I have left a note on my description on all that.
Definitely, let's discuss our options together. In this instance I used the |
|
Thanks for the reply @ParaskP7
strange! Thanks for taking a look into it!
Nice! Sorry I missed that. I'll have a look at the parent task!
Is that the button you're referring to? I was talking about the subheading. We can discuss on our call in a bit 😄 |
|
Thank YOU @osullivanchris !
True, it was just our IDE messing with reordering XML elements automatically, which is causing that, and this is because of the predefined setup we are all using. We also talked with @ashiagr to extract this login in a separate layout and make it reusable. This will avoid further such problems in the future and make it easier for anyone else to reuse this component. 👍
The description was long, of course! 🌟
Oh, you are right, I misread, this is not about the |
This 'Login' hash updates 'Login' library to that 'PR' version where the 'login_or_layout' layout has been extracted and can be now reused.
After talking with the designer it was decided that the best choice is to go with the 'textAppearanceSubtitle1' for now.
|
👋 @osullivanchris !
As discussed, I have just pushed a fix which updates the |
|
Looks good @ParaskP7 and thanks for the chat! I'm updating zeplin to reflect the same change 😄 |
This 'Login' hash updates 'Login' library to that 'develop' version where the 'login_or_layout' layout has been extracted and can be now reused.
ParaskP7
removed
the
[Status] Needs Design Review
Parent #14990
This PR adds a couple of new designs to the login epilogue screen, the
normalandexpandedones.normaldesign is the one that will be shown when the number of sites equal to 2-3 or less.expandeddesign is the one that will be shown when the number of sites end up being more than 3.(
normal)(
expanded)To test:
Scenario:
normalSide Chooserscreen and hide all but 3 of the existing sites. You should at max have no more than 3 sites shown.My Site->App Settings->Test feature configuration.OnboardingImprovementsFeatureConfigfeature flag.Login Epiloguescreen looks as expected, thenormaldesign is shown.Scenario:
expandedSide Chooserscreen and unhide all sites. You should at least have more that 3 sites shown.My Site->App Settings->Test feature configuration.OnboardingImprovementsFeatureConfigfeature flag.Login Epiloguescreen looks as expected, theexpandeddesign is shown.Merge Instructions:
@osullivanchris to do a design review of both the newLogin Epiloguescreens (normalandexpanded).Remove the[Status] Needs Design Review.Merge WordPress-Login-Flow-Android#60.UpdateLoginhash from the PR (60-{sha1}) todevelop-{sha1}.Notes
This PR is UI related only, the below functionality will be handled in separate PRs:
Site, which will either show theQuick Start Promptor navigate toMy Sitedirectly.Create a new sitebutton, which will start theWordPress.comsite creation flow.LandscapeUI, which requires a bit more fine tuning before being ready.Regression Notes
The existing
Login Epiloguescreen might be somehow affected by all the changes in this PR since all 3 screen are sharing all the logic, that is code-wise.See
To testsection above.N/A
PR submission checklist:
RELEASE-NOTES.txtif necessary.