Refactor to decrease code duplication and complexity #301
Conversation
This file contains hidden or 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
Simplify and reduce duplication in the tool-bar-button-view component, specifically in the `setEnabled` and `setSelected` methods by using `classList.toggle()` [1] instead of `classList.add` and `classList.remove` with an if-statement. -- 1. https://developer.mozilla.org/en-US/docs/Web/API/Element/classList#Examples
Update the `updatePosition()` method of the tool-bar-view to set the `tool-bar.position` configuration itself. This presents any issue due to the caller forgetting to set it themselves and reduced code duplication when adding commands for the package.
Refactor the common logic out of the find function into a separate function, making the find logic easier to understand and reduce code duplication.
Add a method to the ToolBarView class to refresh the toolbar. This method is now used when the `tool-bar.position` and `tool-bar.fullWidth` configuration values changed. This aims to reduce code duplication and make it explicit what happens in these anonymous functions.
Simply the `getTooltipPlacement` function by refactoring the nested ternary operators into an object keyed by the toolbar position value.
Refactor the constructor of the ToolBarButtonView class in order to reduce its length and cognitive complexity.
Update the implementation of raf-debounce to reduce it's length and reduce complecity. This is achieve primarily through the removal of if-statements that are "unnecessary" in the sense that everything still works when they're removed. Most notably, there is no longer a check that the requestID is set, as it is safe to call `window.cancelAnimationFrame` with `null` or `undefined`.
Reduce the complexity of the `updatePosition` method of the ToolBarView class primarily by extracting the logic that creates a new panel into a separate method. This already reduced the length of the `updatePosition` method. The length is further reduced by rewriting the logic that sets the classes for the new panel element.
aminya
reviewed
May 28, 2020
aminya
reviewed
May 28, 2020
aminya
reviewed
May 28, 2020
aminya
reviewed
May 28, 2020
aminya
reviewed
May 28, 2020
aminya
reviewed
May 28, 2020
aminya
reviewed
May 28, 2020
suda
approved these changes
Jun 3, 2020
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.
This refactors the code base based on the issues from Code Climate regarding code duplication and code complexity. This should get us from a C to a (not perfect) A 😃
When reviewing this Pull Request, I'd suggest going through the commits one-by-one to see if you agree with every refactoring step that I took. I think that looking at the final diff may be a bit confusing and the motivation for certain refactoring choices may be unclear.