-
Notifications
You must be signed in to change notification settings - Fork 94
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
Ported Sugarizer server's Tour: from Bootstrap to IntroJS #395
base: dev
Are you sure you want to change the base?
Conversation
Great work! I haven't reviewed the code yet, I'm waiting for workflow approval from @llaske. However, there are a few things that can be improved.
|
@llaske can you approve the workflow run on this PR? |
hey @NikhilM98, Thanks for the feedback,I will work on making the necessary changes and will update the issue as soon as possible. |
I'm agree with @NikhilM98 remarks. |
@rockharshitmaurya can you also fix the failing CI checks? |
there are some lint error, to ease this I opened a PR #400 so that contributors will commit after removing lint errors as it is unnecessary efforts to push code and then resolve linting error, because sometime contributors forget to run command |
yeh sure @NikhilM98 sir , I'll be happy to fix that too |
sure @llaske sir, I'll Take Care of That |
hey @llaske @NikhilM98 , I actually started working on this PR and I have a doubt. all the new tour of sugarizer which are using intro.js do not contain an "End" button, they only have an "X" button on the top right bar to end the tour. So, would it be alright to add an "End" button in the server tour, as it would be different from the others? |
@rockharshitmaurya no you're right, no end button should be provided. |
Hi @llaske, is this issue open to work upon? |
@yashveeeeer yes |
Hey, @llaske @NikhilM98 I have ported Sugarizer Server Dashboard Tour from Bootstrap to IntroJS. Please review it.
Refer issue: #363