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

[core/public/navLinks] migrate ui/chrome/navLinks API #23217

Closed

Conversation

spalger
Copy link
Contributor

@spalger spalger commented Sep 16, 2018

Another part of #19992, based on #23341

@spalger spalger added the WIP Work in progress label Sep 16, 2018
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the migrate-new-platform/nav-links branch 2 times, most recently from c6105be to 7f174ba Compare September 17, 2018 15:12
@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the migrate-new-platform/nav-links branch 3 times, most recently from 0d80cad to 3a8b25d Compare September 17, 2018 16:43
@elasticmachine

This comment has been minimized.

@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the migrate-new-platform/nav-links branch 3 times, most recently from 4291a12 to f0f3772 Compare September 18, 2018 06:57
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the migrate-new-platform/nav-links branch 3 times, most recently from 627b317 to ee50248 Compare September 18, 2018 07:26
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the migrate-new-platform/nav-links branch 2 times, most recently from 1c99c94 to 164d7ea Compare September 18, 2018 07:44
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the migrate-new-platform/nav-links branch 2 times, most recently from 8a08266 to 9110e82 Compare September 19, 2018 19:42
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the migrate-new-platform/nav-links branch from 9110e82 to 076a4b0 Compare September 19, 2018 21:24
@spalger spalger force-pushed the migrate-new-platform/nav-links branch 2 times, most recently from 0b6e468 to bf7954c Compare September 19, 2018 22:03
@spalger spalger force-pushed the migrate-new-platform/nav-links branch 2 times, most recently from 1e23418 to 1e5803c Compare September 19, 2018 23:10
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the migrate-new-platform/nav-links branch from 1e5803c to d91acd6 Compare September 19, 2018 23:56
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the migrate-new-platform/nav-links branch from d91acd6 to ac78e17 Compare September 20, 2018 06:44
@elasticmachine

This comment has been minimized.

@spalger spalger force-pushed the migrate-new-platform/nav-links branch from ac78e17 to 50f31b6 Compare September 20, 2018 18:54
@elasticmachine
Copy link
Contributor

💚 Build Succeeded

@spalger
Copy link
Contributor Author

spalger commented Sep 20, 2018

I forgot to port over a pretty important feature here that is going to take some time to get in, and since tomorrow is my last day before vacation I'm going to close this and try to get this in once I'm back.

@spalger spalger closed this Sep 20, 2018
spalger pushed a commit that referenced this pull request Oct 17, 2018
In `ui/public/config/config.js` we have subscription logic that delivers values from an observable synchronously and also ensuring that values are received within a digest cycle. I need to do the same in #23217 but want to keep as few checks for `$rootScope.$$phase` as possible, so I broke the subscription logic into a shared util.

In order to make the utility a little more helpful it will also trigger fatal errors if an observable errors without having an error handler, or if `observer.next()`, `observer.error()`, or `observer.complete()` throw, which would normally be swallowed by RxJS.
spalger pushed a commit to spalger/kibana that referenced this pull request Oct 17, 2018
In `ui/public/config/config.js` we have subscription logic that delivers values from an observable synchronously and also ensuring that values are received within a digest cycle. I need to do the same in elastic#23217 but want to keep as few checks for `$rootScope.$$phase` as possible, so I broke the subscription logic into a shared util.

In order to make the utility a little more helpful it will also trigger fatal errors if an observable errors without having an error handler, or if `observer.next()`, `observer.error()`, or `observer.complete()` throw, which would normally be swallowed by RxJS.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants