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

feat(plugins): implement ability to overwrite elements with slots #1180

Merged
merged 7 commits into from May 3, 2019

Conversation

@luciorubeens
Copy link
Member

commented Apr 10, 2019

Proposed changes

Register a global component to be used by plugins

Types of changes

  • New feature (non-breaking change which adds functionality)
@alexbarnsley

This comment has been minimized.

Copy link
Member

commented Apr 16, 2019

@luciorubeens is there a particular plugin you have available that can be used to test this with? Thanks

@luciorubeens

This comment has been minimized.

Copy link
Member Author

commented Apr 17, 2019

@alexbarnsley Here you go: https://github.com/luciorubeens/ark-desktop-custom-chain. Currently have to access the route to load, but can be improved next.

@alexbarnsley

This comment has been minimized.

Copy link
Member

commented Apr 17, 2019

Looks great! How do you think it could work without having to be on a specific page? e.g. I enable this plugin and the footer is always "My custom chain" while on that profile?

Also, I'm getting a warning about portal-target not being PascalCase:

image

@codecov-io

This comment has been minimized.

Copy link

commented Apr 24, 2019

Codecov Report

Merging #1180 into next will decrease coverage by 0.62%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #1180      +/-   ##
==========================================
- Coverage   39.28%   38.66%   -0.63%     
==========================================
  Files         223      223              
  Lines        5915     5892      -23     
  Branches     1176     1171       -5     
==========================================
- Hits         2324     2278      -46     
- Misses       3393     3416      +23     
  Partials      198      198
Impacted Files Coverage Δ
src/renderer/plugins/plugin-manager.js 0% <0%> (ø) ⬆️
src/renderer/components/Plugin/PluginWrapper.vue 0% <0%> (ø)
src/renderer/components/App/AppFooter.vue 0% <0%> (ø) ⬆️
src/renderer/components/Network/NetworkModal.vue 4.22% <0%> (-24.64%) ⬇️
src/renderer/store/base.js 54.16% <0%> (-4.17%) ⬇️
...actionForm/TransactionFormDelegateRegistration.vue 12.12% <0%> (-1.12%) ⬇️
...TransactionForm/TransactionFormSecondSignature.vue 13.51% <0%> (-0.97%) ⬇️
...action/TransactionForm/TransactionFormTransfer.vue 6.89% <0%> (-0.74%) ⬇️
src/renderer/services/client.js 70.66% <0%> (-0.39%) ⬇️
...ransaction/TransactionForm/TransactionFormVote.vue 10.98% <0%> (-0.25%) ⬇️
... and 22 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 39438a5...09d359c. Read the comment docs.

@luciorubeens

This comment has been minimized.

Copy link
Member Author

commented Apr 24, 2019

Yeh, I thought about loading at startup, does not it look good?

PascalCase problem fixed.

@j-a-m-l j-a-m-l self-assigned this Apr 30, 2019

@j-a-m-l

This comment has been minimized.

Copy link
Contributor

commented Apr 30, 2019

@luciorubeens I just tried it and it works, but, maybe it would be better to rename the PortalTarget to "plugin-footer", but continue exposing it as "footer", so the code of the wallet differentiates clearly which portals are intended to be used internally and which others are there for plugins.

@luciorubeens luciorubeens changed the base branch from v2.4.0 to next May 2, 2019

@luciorubeens luciorubeens requested a review from ItsANameToo as a code owner May 2, 2019

@luciorubeens

This comment has been minimized.

Copy link
Member Author

commented May 2, 2019

Agreed. Done! @j-a-m-l

@j-a-m-l
Copy link
Contributor

left a comment

@luciorubeens now it doesn't work, maybe it's necessary to update the PluginWrapper too.

@alexbarnsley

This comment has been minimized.

Copy link
Member

commented May 3, 2019

@j-a-m-l yes, it needed PluginWrapper to point to the updated portal name. I've made the change if you want to test it.

@j-a-m-l
j-a-m-l approved these changes May 3, 2019

@j-a-m-l j-a-m-l merged commit fca82c4 into next May 3, 2019

1 check passed

ci/circleci: test-node-11 Your tests passed on CircleCI!
Details

@ArkEcosystemBot ArkEcosystemBot deleted the feat/plugin-slot branch May 3, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.