Refactoring error directive #1403
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
## What The event emitter service can't be used in component scope. All events are visible globally and this can lead to misplaced usage between global scope and component scope if events has same names. ## Why The event emitter functionality is globally scoped, and it has to be used for global communication between components only. ## How The "EventEmitterService" was renamed to "GlobalEventBuss" for better semantic meaning. All inner functionality has been moved to a new service "EventEmitterService". When "GlobalEventBuss" is created an instance of EventEmitterService is created.
boyan-tonchev
force-pushed
the
refactoring-error-directive
branch
from
8d2c5ab to
04e82c3
Compare
|
Jenkins retry build |
boyan-tonchev
force-pushed
the
refactoring-error-directive
branch
from
04e82c3 to
8e39fdf
Compare
## What Implements global store. It will hold all shared data for the whole WB. ## Why We want to remove the dependencies between components. This will make the code more extendable and readable. It will also simplify migration to a new version of the framework. ## How A core functionality of component store has been implemented. Its responsibility is to maintains property-value couples. It has functionality to notify all components that are subscribed for some property change event. A globalStoreService has been created. It uses core component store functionality. At this moment, the selected repository object has been moved to the global store. It now takes care of persisting and loading it from the browser store. This will simplify the task of adding the repository ID to the URL.
## What Moved all functionality of active repository reading and persistence to global store. ## Why We want to remove the dependencies between components. This will make the code more extendable and readable. It will also simplify migration to a new version of the framework. ## How All functionality of active repository reading and persistence has been moved to global store.
## What Refactored license service to use the global store. ## Why We want to remove the dependencies between components. This will make the code more extendable and readable. It will also simplify migration to a new version of the framework. ## How Injects GlobalStoreService into LicenseService and refactoring all inner logic to use it.
…nality; ## What Reorganized the code of core error directive without changing functionality. ## Why For better reading and futture refactoring. ## How Reorganized the code of core error directive without changing functionality.
## What Refactored core error directive. ## Why - There was a brief display of the error template when refreshing some views; - We want to remove the dependencies between components. This will make the code more extendable and readable. It will also simplify migration to a new version of the framework. ## How The core error directive was moved in separate file. Its scope was changed to isolate. Creates core error store that is used where is needed in components that used directive.
boyan-tonchev
force-pushed
the
refactoring-error-directive
branch
from
8e39fdf to
ec531de
Compare
|
| /** | ||
| * A service responsible for emitting events between components. It takes care to registering subscribers and calling them when an event for which they are registered occurs. | ||
| */ | ||
| function GlobalEmitterBuss() { |
There was a problem hiding this comment.
This is not an event bus implementation but a simple event emitter or pub/sub
| // makes directive isolated scope | ||
| scope: {}, | ||
| templateUrl: 'js/angular/core/directives/core-error/templates/core-errors.html', | ||
| link: function (scope, element, attrs) { |
There was a problem hiding this comment.
Add the dollar sign in front of the scope. It's kind of convention.
| }, Promise.resolve(eventData)); | ||
|
|
||
| eventSubscribersChain.then(() => { | ||
| if (angular.isFunction(callback)) { |
There was a problem hiding this comment.
try to avoid using these builtin angular utility functions. There is no alternative in Angular2> for them. Better use vanillajs or lodash.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Not ready yet. Just for run the test.