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

Fix settings survey route not accessible from mode-context #1692

Merged
merged 4 commits into from Aug 4, 2021

Conversation

Ifycode
Copy link
Member

@Ifycode Ifycode commented Jul 25, 2021

This pull request makes the following changes

Details

  • Settings module is lazy loaded depending on where user clicks first (mode-bar or mode-context link)

Screenshots

Screenshot 2021-07-23 at 6 05 13 AM Screenshot 2021-07-27 at 7 18 44 PM

Testing checklist

  • Settings module is lazy loaded when "settings" link is clicked on from the mode-bar
  • Settings module is lazy loaded when "create new survey" link is clicked on from the mode-context
  • Settings module is lazy loaded when "survey settings" button/link is clicked on, from the small dropdown in the mode-context
  • I certify that I ran my checklist

Ping @ushahidi/platform

@Ifycode Ifycode requested a review from Angamanga July 25, 2021 20:27
@Ifycode
Copy link
Member Author

Ifycode commented Jul 25, 2021

@Angamanga can you take a look at this pull request when you have time?

@Ifycode
Copy link
Member Author

Ifycode commented Jul 27, 2021

@Angamanga Another place in the mode-context doesn't work. I've fixed it too (screenshots above). Ready for review when you're back.

Copy link
Collaborator

@Angamanga Angamanga left a comment

Choose a reason for hiding this comment

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

Break out lazy-load function, then approved!

@Ifycode
Copy link
Member Author

Ifycode commented Aug 2, 2021

I've made the change @Angamanga. So moving all those routes from settings-list.routes into the settings.routes file works, but the size for settings bundle is reduced from 1.5mb to about 700kb. The size of the ushahidi-legacy-app bundle is back up to 11.2mb. That's what I observed.

@Angamanga
Copy link
Collaborator

@Ifycode Ok lets go with the original plan for now and skip my idea :D Sorry for the confusion...

@Ifycode
Copy link
Member Author

Ifycode commented Aug 4, 2021

I've pushed a new change where I merged the two ideas @Angamanga you can check and test it now.

Copy link
Collaborator

@Angamanga Angamanga left a comment

Choose a reason for hiding this comment

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

Approved!

url: '/settings',
template: require('./settings-list.html'),
controller: require('./settings-list.controller.js'),
template: require('./settings.html'),
lazyLoad: function ($transition$) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nice solution!

@Angamanga Angamanga merged commit 99cf1bf into ushahidi:feature/migration Aug 4, 2021
@Ifycode Ifycode deleted the fix-settings-survey branch August 4, 2021 11:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants