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.

Sign up for GitHub

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

Jump to bottom

Decouple action buttons from the editor. Expose action functions #44

Merged
merged 8 commits into from
Aug 23, 2023
Merged

Decouple action buttons from the editor. Expose action functions #44

merged 8 commits into from
Aug 23, 2023

Conversation

beliolfa
Copy link
Contributor

@beliolfa beliolfa commented Aug 22, 2023 •
edited
Loading

  • Add optional prop to hideActionButtons
  • Expose functions to perform the same actions that Action Buttons do from outside
  • Refactor validEditor computed to be part of the store so it can be reused
  • Expose two-way bindings to know the state of isSigned and isValid

Closes #39

@beliolfa beliolfa requested a review from samlown August 22, 2023 14:26
@netlify
Copy link

netlify bot commented Aug 22, 2023 •
edited
Loading

✅ Deploy Preview for gobl-build ready!

Name Link
🔨 Latest commit 4d6affb
🔍 Latest deploy log https://app.netlify.com/sites/gobl-build/deploys/64e6125e36d86100080cc106
😎 Deploy Preview https://deploy-preview-44--gobl-build.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

samlown
samlown reviewed Aug 22, 2023
View reviewed changes
Copy link
Contributor

@samlown samlown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good from the outset! But I can't help thinking this is a good opportunity to refactor the Builder and de-couple the actions from the UI buttons.

src/lib/GOBLBuilder.svelte Outdated Show resolved Hide resolved
src/lib/GOBLBuilder.svelte Outdated Show resolved Hide resolved
@beliolfa beliolfa requested a review from samlown August 23, 2023 08:54
samlown
samlown approved these changes Aug 23, 2023
View reviewed changes
Copy link
Contributor

@samlown samlown left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking great! I've added some suggestions to consider as well.

src/lib/editor/stores.ts Outdated Show resolved Hide resolved
@beliolfa beliolfa requested a review from samlown August 23, 2023 13:34
@beliolfa beliolfa requested a review from samlown August 23, 2023 14:01
@beliolfa beliolfa merged commit 410e78a into main Aug 23, 2023
7 checks passed
@beliolfa beliolfa deleted the decouple_buttons branch June 24, 2024 12:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cavalle cavalle cavalle left review comments

@samlown samlown Awaiting requested review from samlown

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Move actions *outside* of builder
3 participants
@beliolfa @cavalle @samlown