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

Added Linear stepper and custom step actions #3641

Conversation

Bond-Addict
Copy link
Contributor

Description

closes #3635
This change will introduce two useful features:

  • Linear Mode- This forces form validation to be valid before navigation will occur.

  • Custom step actions- This add a save property on the Step interface. This allows the user to specify save actions for each step. This goes hand in hand with linear because it makes it where the stepper can be treated like a true wizard ensuring a previous step has completed successfully. Example: You create a user on step 1 and then you assign multiple roles on step 2. If the user didn't exist when you get to step 2 you wouldn't be able to add the roles. You won't navigate to the desired step unless the save function succeeds.

  • Required validator- Adds a required function that accepts a custom error message but provides a default one.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • Improvement/refactoring (non-breaking change that doesn't add any feature but make things better)

@Bond-Addict
Copy link
Contributor Author

The whitespace changes where made automatically by prettier

@m0ksem m0ksem self-requested a review July 28, 2023 20:43
@m0ksem
Copy link
Collaborator

m0ksem commented Jul 29, 2023

The whitespace changes where made automatically by prettier

I understand it. But better do not use any formatter, so it is easier to review the code...

Maybe we should have a standardized Prettier config, actually.

Copy link
Collaborator

@m0ksem m0ksem left a comment

Choose a reason for hiding this comment

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

The package-lock file must be removed, because we use yarn.

packages/ui/package.json Outdated Show resolved Hide resolved
packages/ui/src/components/va-stepper/VaStepper.vue Outdated Show resolved Hide resolved
packages/ui/src/components/va-stepper/VaStepper.vue Outdated Show resolved Hide resolved
@m0ksem m0ksem added the feature Something useful to end user label Jul 29, 2023
@Bond-Addict
Copy link
Contributor Author

Bond-Addict commented Aug 1, 2023

I added an invalid class for the stepper step too.
image001 (2).png

@Bond-Addict
Copy link
Contributor Author

The only conflict I have is the yarn lock. Should I do the same as the package.lock and delete the yarn.lock?

@m0ksem
Copy link
Collaborator

m0ksem commented Aug 1, 2023

The only conflict I have is the yarn lock. Should I do the same as the package.lock and delete the yarn.lock?

I can handle it before merge.

@m0ksem m0ksem self-requested a review August 1, 2023 17:15
@m0ksem m0ksem force-pushed the feat/stepper-with-save-actions-and-linear-mode branch from 28c990e to 9241045 Compare August 11, 2023 17:44
@m0ksem
Copy link
Collaborator

m0ksem commented Aug 11, 2023

@Bond-Addict, I made demos less confusing and also added few extra features. Please, review if I did something wrong. Otherwise, it looks good for me. The last step is to update docs demo if you're ok with changes I made.

@Bond-Addict
Copy link
Contributor Author

@Bond-Addict, I made demos less confusing and also added few extra features. Please, review if I did something wrong. Otherwise, it looks good for me. The last step is to update docs demo if you're ok with changes I made.

I looks great dude. Thanks for the assist! Just for my understanding, what was I doing wrong with the button color part? Was there some vue fundamental I was messing up? I'll get the docs updated as soon as possible.

@m0ksem
Copy link
Collaborator

m0ksem commented Aug 11, 2023

@Bond-Addict, I made demos less confusing and also added few extra features. Please, review if I did something wrong. Otherwise, it looks good for me. The last step is to update docs demo if you're ok with changes I made.

I looks great dude. Thanks for the assist! Just for my understanding, what was I doing wrong with the button color part? Was there some vue fundamental I was messing up? I'll get the docs updated as soon as possible.

There was a bug on line https://github.com/epicmaxco/vuestic-ui/pull/3641/files#diff-330631d4f7a4d81d8d3d14e31f746d0e1c148d4e5bb9c063913166c6a7b5addeL50. Because it wasn't computed, color wasn't updating if you change it later... So it always was primary.

@Bond-Addict
Copy link
Contributor Author

@Bond-Addict, I made demos less confusing and also added few extra features. Please, review if I did something wrong. Otherwise, it looks good for me. The last step is to update docs demo if you're ok with changes I made.

I looks great dude. Thanks for the assist! Just for my understanding, what was I doing wrong with the button color part? Was there some vue fundamental I was messing up? I'll get the docs updated as soon as possible.

There was a bug on line https://github.com/epicmaxco/vuestic-ui/pull/3641/files#diff-330631d4f7a4d81d8d3d14e31f746d0e1c148d4e5bb9c063913166c6a7b5addeL50. Because it wasn't computed, color wasn't updating if you change it later... So it always was primary.

Ah okay. I had tried to use a computed earlier but there must have been something messed up. I'm glad to know I was on the right track though. I'll try and get the docs update Monday morning. Have a good weekend!

@Bond-Addict
Copy link
Contributor Author

@m0ksem Can you explain the correct steps to get the docs to run? I've gotten it to generate one time, but I have no idea how I did it. Once I can get them to run I'll get the docs updated. I've been looking at the contribution docs, but cant seem to figure it out. I might be doing something wrong, but I cant get yarn serve:docs to work in either docs or ui.

@m0ksem
Copy link
Collaborator

m0ksem commented Aug 15, 2023

@m0ksem Can you explain the correct steps to get the docs to run? I've gotten it to generate one time, but I have no idea how I did it. Once I can get them to run I'll get the docs updated. I've been looking at the contribution docs, but cant seem to figure it out. I might be doing something wrong, but I cant get yarn serve:docs to work in either docs or ui.

Maybe you need to reinstall deps. Run yarn && yarn serve:docs in root of the project and you should be good.

@Bond-Addict
Copy link
Contributor Author

I this error normal?

Failed to resolve import "fsevents" from "node_modules\.cache\vite\client\deps\chunk-5CLDZUOX.js?v=3a618fd0". Does the file exist?

@m0ksem
Copy link
Collaborator

m0ksem commented Aug 15, 2023

I this error normal?

Failed to resolve import "fsevents" from "node_modules\.cache\vite\client\deps\chunk-5CLDZUOX.js?v=3a618fd0". Does the file exist?

Nope. Try to clean node_modules/.cache or .nuxt folder. Likely something is cached. Or try yarn --force.

@Bond-Addict
Copy link
Contributor Author

I this error normal?
Failed to resolve import "fsevents" from "node_modules\.cache\vite\client\deps\chunk-5CLDZUOX.js?v=3a618fd0". Does the file exist?

Nope. Try to clean node_modules/.cache or .nuxt folder. Likely something is cached. Or try yarn --force.

I'll try clearing cache and nuxt folder but I have tried a couple times to run yarn --force

@asvae asvae self-requested a review August 17, 2023 10:45
@Bond-Addict
Copy link
Contributor Author

Author

@m0ksem I'm unable to get the docs updated due to this issue. In light of this, would you be able to update the doc? My project is dependent on this feature so its at a stand standstill.

@m0ksem
Copy link
Collaborator

m0ksem commented Aug 17, 2023

Author

@m0ksem I'm unable to get the docs updated due to this issue. In light of this, would you be able to update the doc? My project is dependent on this feature so its at a stand standstill.

Yeah, sure. I wanted to handle the rest of pr before next release and I hope @asvae gonna have some time to review it.

@Bond-Addict
Copy link
Contributor Author

Author

@m0ksem I'm unable to get the docs updated due to this issue. In light of this, would you be able to update the doc? My project is dependent on this feature so its at a stand standstill.

Yeah, sure. I wanted to handle the rest of pr before next release and I hope @asvae gonna have some time to review it.

Awesome, thank you so much. I appreciate the continued support. I'll try and ensure the next change goes a bit smoother.

Copy link
Member

@asvae asvae left a comment

Choose a reason for hiding this comment

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

Thanks!

This looks good enough for now, enabling form functionality for stepper. Later we might want to rework the component - some things are not done optimally.

@Bond-Addict
Copy link
Contributor Author

@m0ksem How do I resolve your requested change? The pr is currently failing the "Changes requested" precheck.

@asvae
Copy link
Member

asvae commented Aug 25, 2023

@Bond-Addict hey, I actually coded it quickly myself (hope you don't mind the intrusion 🙏).

I can merge it no problems. :)

@Bond-Addict
Copy link
Contributor Author

@Bond-Addict hey, I actually coded it quickly myself (hope you don't mind the intrusion 🙏).

I can merge it no problems. :)

I don't mind at all. I appreciate the assist from yall to get this feature in. I look forward to seeing the future optimizations as I'm still learning Vue and want to learn the best practices 😃

@m0ksem
Copy link
Collaborator

m0ksem commented Aug 25, 2023

@m0ksem How do I resolve your requested change? The pr is currently failing the "Changes requested" precheck.

Hi, we good. I'm not sure if you saw that I pinged you in Discord, so you could get this feature released in an experimental build.

We're currently in a process to move dev discussions to discord, where we can communicate faster with contributors, help them and ship code faster. If you were interested in real-time communication instead of GitHub comments, you're welcome!

  • We about to release it, but it actually was in version 0.0.0-experimental-5d49247e6-20230821 already.

@Bond-Addict
Copy link
Contributor Author

@m0ksem How do I resolve your requested change? The pr is currently failing the "Changes requested" precheck.

Hi, we good. I'm not sure if you saw that I pinged you in Discord, so you could get this feature released in an experimental build.

We're currently in a process to move dev discussions to discord, where we can communicate faster with contributors, help them and ship code faster. If you were interested in real-time communication instead of GitHub comments, you're welcome!

  • We about to release it, but it actually was in version 0.0.0-experimental-5d49247e6-20230821 already.

Ah okay I must have missed that message. Thanks for all the assistance getting this out. You rock! 😄

@asvae asvae merged commit 0998147 into epicmaxco:develop Aug 26, 2023
m0ksem added a commit that referenced this pull request Sep 12, 2023
* Added Linear stepper and custom step actions

* Reverted prettier changes and removed package-lock

* Wip for pr comments

* Passing form to stepper wip

* Trying to pass ref as a nested object to protect against unwrapping

* fix(va-stepper): register va-form component in demos

* fix(va-stepper): memory leak using useForm in methods

* fix(va-stepper): remove extra FormRef type

* Working on form ref reactivity

* Cleanup

* Set prop default and add another field to demo

* Set navigation buttons disabled if invalid

* Set invalid color on invalid form

* Package updates

* Docs update

* Changed stepper to not handle validating form. Linear does work as intended. Steps must be manually completed if linear prop is provided.

* Cleanup

* Whitespace fixes

* More whitespace fixes

* More whitespace fixes

* More whitespace fixes and doc updates

* More whitespace fixes

* More whitespace fixes

* More whitespace fixes

* More whitespace fixes

* Whitespace

* Whitespace

* Whitespace

* Whitespace

* Whitespace

* Whitespace

* chore: update yarn lock

* Suggestions Pt.1

* Trying to set danger if error exists

* Trying to fix error color. It seems like it should work, but the step button isnt reacting to change.

* fix(stepper): fix: refactor stepper validation

* feat(stepper): added computed error

* docs(stepper): add linear docs demo

* chore: beforeSave -> beforeLeave

---------

Co-authored-by: Maksim Nedoshev <m0ksem1337@gmail.com>
Co-authored-by: Yauheni Prakopchyk <yauheni.prakopchyk@epicmax.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature Something useful to end user
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Added linear (validation dependent) option for VaStepper
3 participants