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

🎨 Code clean-up #139

Merged
merged 18 commits into from
Jan 31, 2023
Merged

🎨 Code clean-up #139

merged 18 commits into from
Jan 31, 2023

Conversation

joao-m-santos
Copy link
Contributor

@joao-m-santos joao-m-santos commented Jan 17, 2023

Relates to #130

πŸ“ Description

1643ad0 βœ… Fix test failure (wrong import)
982a647 🎨 Update prop composables & docs data
bc47d57 βœ… Fix unit tests
1afa447 ♻️ Abstract lume-alluvial-path-group to lume-alluvial-group
b8f810a ♻️ Refactor Alluvial diagram and its sub-elements
c0463c4 🎨 Remove duplicate DEFAULT_COLOR variable
b214a09 βœ… Add group-props spec test file
2b8d40e 🚚 Move group-props to @/composables
fc656db βœ… Fix failing test
ea601ef 🚚 Move path-group into alluvial-group folder
8083216 🩹 Fix chart legend stories
dfb5f00 ♻️ Refactor lume-point API; Add stories; Update docs
a4089b1 ♻️ Refactor group components into script setup
8468a02 🎨 Remove Vue.set; Improve chartType; Add title color vars
0064b59 πŸ“ Add lume-line stories; Update documentation
6cad45f ♻️ Move transition logic to line-group
85bdbb2 🎨 Remove Vue import and Vue.set usage
55bf875 πŸ’„ Remove color from alluvial end node

πŸ’₯ Is this a breaking change (Yes/No):

  • No
  • Yes (please describe the impact and migration path for existing Lume users)

πŸ“ Additional Information

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@joao-m-santos joao-m-santos linked an issue Jan 17, 2023 that may be closed by this pull request
@netlify
Copy link

netlify bot commented Jan 17, 2023

❌ Deploy Preview for adyen-lume failed.

Name Link
πŸ”¨ Latest commit 1643ad0
πŸ” Latest deploy log https://app.netlify.com/sites/adyen-lume/deploys/63d8e2b24d1be00008f21fec

@joao-m-santos joao-m-santos changed the title πŸ’„ Remove color from alluvial end node 🎨 Code clean-up Jan 17, 2023
* Removed unused computed prop
* Improved imports
* Use group props instead of chart props
* Also some minor code improvements
* Renamed the alluvial group composables
* Simplified composable code and their responsibilities
* Moved alluvialInstance properties to be options (better
  aligned with other charts work)
* Added new AlluvialDiagramOptions type
* Simplified alluvial specific typings
* Added new errors composable to log chart errors
* Improved alluvial diagram performance
  * Decreased re-render count
  * Decreased overall function calls
@joao-m-santos joao-m-santos self-assigned this Jan 31, 2023
@joao-m-santos joao-m-santos marked this pull request as ready for review January 31, 2023 10:07
@joao-m-santos joao-m-santos requested a review from a team as a code owner January 31, 2023 10:07
@vivy27 vivy27 self-requested a review January 31, 2023 12:30
@joao-m-santos joao-m-santos merged commit b2f4025 into main Jan 31, 2023
@joao-m-santos joao-m-santos deleted the 130-code-clean-up branch September 1, 2023 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Code clean-up
2 participants