POC: Replace foundation joyride with Driver.js#3471
Conversation
Test results 12 files 12 suites 12m 9s ⏱️ Results for commit 9f34f44. ♻️ This comment has been updated with latest results. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3471 +/- ##
==========================================
+ Coverage 61.02% 61.04% +0.02%
==========================================
Files 610 610
Lines 44707 44708 +1
Branches 43 43
==========================================
+ Hits 27282 27294 +12
+ Misses 17415 17404 -11
Partials 10 10 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| <div class="row"> | ||
| {% if request.account.is_admin %} | ||
| <div class="column medium-6"> | ||
| <a id="joyrideme" href="javascript:void(0);" |
There was a problem hiding this comment.
Never was a fan of "javascript:void(0);", w00t!
There was a problem hiding this comment.
Yeah... why not just use the element instead 🤯
There was a problem hiding this comment.
I hope we don't use all of these. Is there a way to shorten this list, remove the ones we definitely don't use? Makes the task less daunting.
There was a problem hiding this comment.
I hope we can remove as many as possible as we replace some of the js-enabled foundation components. It's going to be super fun and barely an inconvenience, I'm sure
There was a problem hiding this comment.
No doubt some are dependent on each other...
8e5063f to
c499097
Compare
|
Thanks for the screen shots. NAV is old-fashionedly, extremely, square, while Driver.js is subtly curved all over. These helper-wizards are so separate from the rest of the UI that that might be ok but I'd like @lunkwill42's opinion on the looks. Another change is that the actual text is moved from templates to static files, from html to js. I think that warrants a note in the docs somewhere. (The real challenge with docs in NAV is knowing where to put stuff and what to call the files, and it is a real challenge =) ) |
I've tried adjusting the styles, and extending the existing buttons, with this result (using dark mode, matching the existing Joyride almost exactly): With just changing of border radius and using Nav buttons in light mode, it looks like this: |
c499097 to
c2f396d
Compare
c2f396d to
1ed8dea
Compare
johannaengland
left a comment
There was a problem hiding this comment.
I might be forgetting something here, but when trying to test this in my devcontainer I get an empty front page (none of my widgets are shown) and am not able to add the widget for the tour:
And in the background I get the following error:
"GET /static/js/driver.js?bust=5.14.1.dev7+g1ed8dea0d HTTP/1.1" 404 1937
|
Both of these are better. They're missing from the styleguide so we should add one there as well. |
hmpf
left a comment
There was a problem hiding this comment.
Let's go with white background, standard NAV buttons (the last screenshot).
Scope and purpose
Closes #3468. Replaces foundation joyride usages with implementations in Driver.js.
I chose this library because I liked the API, and the library is the smallest I found and without external dependencies.
The old popups were dark with white text, while the defaults in Driver.js is white with dark text. In my opinion, this works better when the rest of the app is light, and I therefore opted to not implement the same styles as the existing Joyride.
Add a description of what this PR does and why it is needed. If a linked ticket(s) fully
cover it, you can omit this.
POC because it is a new library, and others might disagree with the choice or want alternatives.
Screenshots
Before
Getting Started
Add Netbox
After
Getting Started
Add Netbox
This pull request
Contributor Checklist
<major>.<minor>.x). For a new feature or other additions, it should be based onmaster.