Bug 38899 - Allow the Vue toolbar to be sticky
Summary: Allow the Vue toolbar to be sticky
Status: Pushed to main
Alias: None
Product: Koha
Classification: Unclassified
Component: ERM (show other bugs)
Version: Main
Hardware: All All
: P5 - low normal
Assignee: Matt Blenkinsop
QA Contact: Testopia
URL:
Keywords:
Depends on: 38010 40067
Blocks:
  Show dependency treegraph
 
Reported: 2025-01-15 10:14 UTC by Matt Blenkinsop
Modified: 2025-06-11 20:07 UTC (History)
9 users (show)

See Also:
GIT URL:
Change sponsored?: ---
Patch complexity: ---
Documentation contact:
Documentation submission:
Text to go in the release notes:
This restores the sticky toolbar when adding a vendor in the acquisitions module (Acquisitions > + New vendor). This is related to bug 38010, which migrates vendors in the acquisitions module to using Vue - the sticky menu was not included in this.
Version(s) released in:
25.11.00
Circulation function:


Attachments
Bug 38899: Add a sticky toolbar (5.28 KB, patch)
2025-01-15 10:42 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Add a cypress test (1.15 KB, patch)
2025-01-15 10:42 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Add a sticky toolbar (5.52 KB, patch)
2025-05-01 07:40 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Add a cypress test (1.14 KB, patch)
2025-05-01 07:40 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Add a sticky toolbar (5.56 KB, patch)
2025-05-02 11:31 UTC, David Nind
Details | Diff | Splinter Review
Bug 38899: Add a cypress test (1.18 KB, patch)
2025-05-02 11:31 UTC, David Nind
Details | Diff | Splinter Review
Bug 38899: (QA follow-up) Match buttons to patron toolbar (2.87 KB, patch)
2025-05-06 10:35 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: (QA follow-up) Match buttons to patron toolbar (2.88 KB, patch)
2025-05-09 07:28 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Fix cypress tests (2.35 KB, patch)
2025-05-12 07:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38899: Move the toolbar out of the form (2.97 KB, patch)
2025-05-12 07:58 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38899: Improve HTML consistency (4.54 KB, patch)
2025-05-12 10:43 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Remove code duplication (2.27 KB, patch)
2025-05-14 14:04 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38899: Restore id attribute to the form (1.46 KB, patch)
2025-05-19 10:10 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Fix cypress tests (2.34 KB, patch)
2025-05-20 08:39 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Add a sticky toolbar (5.57 KB, patch)
2025-06-05 09:12 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Add a cypress test (1.18 KB, patch)
2025-06-05 09:12 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: (QA follow-up) Match buttons to patron toolbar (2.88 KB, patch)
2025-06-05 09:12 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Move the toolbar out of the form (2.97 KB, patch)
2025-06-05 09:13 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Improve HTML consistency (4.54 KB, patch)
2025-06-05 09:13 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Remove code duplication (2.27 KB, patch)
2025-06-05 09:13 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Restore id attribute to the form (1.46 KB, patch)
2025-06-05 09:13 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Fix cypress tests (1.64 KB, patch)
2025-06-05 09:13 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 38899: Add a sticky toolbar (5.64 KB, patch)
2025-06-10 09:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38899: Add a cypress test (1.26 KB, patch)
2025-06-10 09:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38899: (QA follow-up) Match buttons to patron toolbar (2.95 KB, patch)
2025-06-10 09:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38899: Move the toolbar out of the form (3.04 KB, patch)
2025-06-10 09:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38899: Improve HTML consistency (4.61 KB, patch)
2025-06-10 09:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38899: Remove code duplication (2.34 KB, patch)
2025-06-10 09:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38899: Restore id attribute to the form (1.53 KB, patch)
2025-06-10 09:31 UTC, Jonathan Druart
Details | Diff | Splinter Review
Bug 38899: Fix cypress tests (1.71 KB, patch)
2025-06-10 09:32 UTC, Jonathan Druart
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Matt Blenkinsop 2025-01-15 10:14:08 UTC
We have sticky toolbars throughout Koha but this is not yet supported in Vue apps. The Toolbar component should have a sticky mode enabled
Comment 1 Matt Blenkinsop 2025-01-15 10:42:40 UTC
Created attachment 176545 [details] [review]
Bug 38899: Add a sticky toolbar

This patch adds a sticky toolbar to the vendors form

Test plan:
N.B: This patchset will not work on a sandbox

1) Navigate to Acquisitions and click New vendor
2) The toolbar should appear at the top of the page underneath Add vendor
3) Scroll down, the toolbar should stick and be given the 'floating' class
4) Scroll back up and the toolbar should unstick and lose the 'floating' class
Comment 2 Matt Blenkinsop 2025-01-15 10:42:42 UTC
Created attachment 176546 [details] [review]
Bug 38899: Add a cypress test
Comment 3 Brendan Gallagher 2025-04-02 07:58:33 UTC
error: sha1 information is lacking or useless (koha-tmpl/intranet-tmpl/prog/js/vue/components/Vendors/VendorFormAdd.vue).
error: could not build fake ancestor

Could be my problem?
Comment 4 Matt Blenkinsop 2025-04-02 08:10:28 UTC
Needs a rebase! I'll pick that up when I get a chance
Comment 5 Jonathan Druart 2025-04-30 19:57:03 UTC
We need that for 25.05 IMO, it's a regression (on the vendor edit page).
Comment 6 Matt Blenkinsop 2025-05-01 07:40:32 UTC
Created attachment 181822 [details] [review]
Bug 38899: Add a sticky toolbar

This patch adds a sticky toolbar to the vendors form

Test plan:
N.B: This patchset will not work on a sandbox

1) Navigate to Acquisitions and click New vendor
2) The toolbar should appear at the top of the page underneath Add vendor
3) Scroll down, the toolbar should stick and be given the 'floating' class
4) Scroll back up and the toolbar should unstick and lose the 'floating' class
Comment 7 Matt Blenkinsop 2025-05-01 07:40:35 UTC
Created attachment 181823 [details] [review]
Bug 38899: Add a cypress test
Comment 8 David Nind 2025-05-01 22:46:50 UTC
What is required to make this work so I can test it?

At the moment, after applying bug 38010 and doing a yarn build, restart_all, and using a browser with the cache cleared, I get a 404 page not found when going to Acquisitions > +New vendor.

It might be obvious if you are a Vue developer what is required, but not to me 8-).

Testing notes so far (using KTD):
1. This bug is related to bug 38010 - migrating vendors in the acquisitions module to using Vue.
2. Before bug 38010, the toolbar when adding a vendor (Acquisitions > + New vendor) was sticky.
3. After bug 38010, the toolbar is no longer sticky, as there is no sticky component/code in the Koha Vue code - this bug adds that. (My understanding so far.)
4. Before applying any patches, go to add a vendor in the acquisitions module (Acquisitions > + New vendor).
5. Note that as you scroll down the page, a sticky menu appears.
6. Apply the patches (and dependencies) for bug 38010.
7. Rebuild things: yarn build
8. Restart everything: restart_all
Comment 9 Matt Blenkinsop 2025-05-02 07:55:26 UTC
Hi David, there is an apache redirect that needs to be in place - a reset_all will do this for you
Comment 10 David Nind 2025-05-02 11:27:59 UTC
(In reply to Matt Blenkinsop from comment #9)
> Hi David, there is an apache redirect that needs to be in place - a
> reset_all will do this for you

Thanks Matt!
Comment 11 David Nind 2025-05-02 11:31:53 UTC
Created attachment 181851 [details] [review]
Bug 38899: Add a sticky toolbar

This patch adds a sticky toolbar to the vendors form

Test plan:
N.B: This patchset will not work on a sandbox

1) Navigate to Acquisitions and click New vendor
2) The toolbar should appear at the top of the page underneath Add vendor
3) Scroll down, the toolbar should stick and be given the 'floating' class
4) Scroll back up and the toolbar should unstick and lose the 'floating' class

Signed-off-by: David Nind <david@davidnind.com>
Comment 12 David Nind 2025-05-02 11:31:57 UTC
Created attachment 181852 [details] [review]
Bug 38899: Add a cypress test

Signed-off-by: David Nind <david@davidnind.com>
Comment 13 David Nind 2025-05-02 11:40:47 UTC
Testing notes (using KTD):
1. This bug is related to bug 38010 - migrating vendors in the acquisitions module to using Vue.
2. Before bug 38010, the toolbar when adding a vendor (Acquisitions > + New vendor) was sticky.
3. After bug 38010, the toolbar is no longer sticky, as there is no sticky component/code in the Koha Vue code - this bug adds that.
4. Before applying any patches, go to add a vendor in the acquisitions module (Acquisitions > + New vendor).
5. Note that as you scroll down the page, a sticky menu appears.
6. Apply the patches (and dependencies) for bug 38010.
7. Reset everything (required for an Apache redirect): reset_all
8. Go to add a vendor in the acquisitions module (Acquisitions > + New vendor).
7. Note that as you scroll down the page, the sticky menu in step 5 no longer appears.
8. Apply the patches for this bug (38899).
9. Rebuild things: yarn build
10. Restart everything: restart_all
11. Go to add a vendor in the acquisitions module (Acquisitions > + New vendor).
12. Note that the sticky toolbar is back!
Comment 14 David Nind 2025-05-02 11:44:37 UTC
13. The tests should pass: yarn cypress run --spec t/cypress/integration/Toolbar_spec.ts
Comment 15 Paul Derscheid 2025-05-02 13:33:41 UTC
Is this just me being dumb?
jquery-migrate-3.3.2.min_24.1200036.js:2 JQMIGRATE: Migrate is installed, version 3.3.2
flatpickr.min_24.1200036.js:2 TypeError: Cannot read properties of undefined (reading 'appendChild')
    at Object.onReady (shortcut-buttons-flatpickr.min_24.1200036.js:2:3024)
    at pe (flatpickr.min_24.1200036.js:2:34535)
    at flatpickr.min_24.1200036.js:2:48317
    at E (flatpickr.min_24.1200036.js:2:48331)
    at k (flatpickr.min_24.1200036.js:2:48618)
    at jQuery.fn.jQuery.fn.flatpickr (flatpickr.min_24.1200036.js:2:49426)
    at apply_flatpickr (acqui-home.pl:1585:7166)
    at HTMLInputElement.<anonymous> (acqui-home.pl:1585:7823)
    at S.each (jquery-3.6.0.min_24.1200036.js:2:3003)
    at s.fn.init.each (jquery-3.6.0.min_24.1200036.js:2:1481)
k @ flatpickr.min_24.1200036.js:2
jQuery.fn.jQuery.fn.flatpickr @ flatpickr.min_24.1200036.js:2
apply_flatpickr @ acqui-home.pl:1585
(anonymous) @ acqui-home.pl:1585
each @ jquery-3.6.0.min_24.1200036.js:2
each @ jquery-3.6.0.min_24.1200036.js:2
(anonymous) @ acqui-home.pl:1585
e @ jquery-3.6.0.min_24.1200036.js:2
t @ jquery-3.6.0.min_24.1200036.js:2
setTimeout
(anonymous) @ jquery-3.6.0.min_24.1200036.js:2
c @ jquery-3.6.0.min_24.1200036.js:2
fireWith @ jquery-3.6.0.min_24.1200036.js:2
fire @ jquery-3.6.0.min_24.1200036.js:2
c @ jquery-3.6.0.min_24.1200036.js:2
fireWith @ jquery-3.6.0.min_24.1200036.js:2
ready @ jquery-3.6.0.min_24.1200036.js:2
B @ jquery-3.6.0.min_24.1200036.js:2
flatpickr.min_24.1200036.js:2 TypeError: Cannot read properties of undefined (reading 'appendChild')
    at Object.onReady (shortcut-buttons-flatpickr.min_24.1200036.js:2:3024)
    at pe (flatpickr.min_24.1200036.js:2:34535)
    at flatpickr.min_24.1200036.js:2:48317
    at E (flatpickr.min_24.1200036.js:2:48331)
    at k (flatpickr.min_24.1200036.js:2:48618)
    at jQuery.fn.jQuery.fn.flatpickr (flatpickr.min_24.1200036.js:2:49426)
    at apply_flatpickr (acqui-home.pl:1585:7166)
    at HTMLInputElement.<anonymous> (acqui-home.pl:1585:7823)
    at S.each (jquery-3.6.0.min_24.1200036.js:2:3003)
    at s.fn.init.each (jquery-3.6.0.min_24.1200036.js:2:1481)
k @ flatpickr.min_24.1200036.js:2
jQuery.fn.jQuery.fn.flatpickr @ flatpickr.min_24.1200036.js:2
apply_flatpickr @ acqui-home.pl:1585
(anonymous) @ acqui-home.pl:1585
each @ jquery-3.6.0.min_24.1200036.js:2
each @ jquery-3.6.0.min_24.1200036.js:2
(anonymous) @ acqui-home.pl:1585
e @ jquery-3.6.0.min_24.1200036.js:2
t @ jquery-3.6.0.min_24.1200036.js:2
setTimeout
(anonymous) @ jquery-3.6.0.min_24.1200036.js:2
c @ jquery-3.6.0.min_24.1200036.js:2
fireWith @ jquery-3.6.0.min_24.1200036.js:2
fire @ jquery-3.6.0.min_24.1200036.js:2
c @ jquery-3.6.0.min_24.1200036.js:2
fireWith @ jquery-3.6.0.min_24.1200036.js:2
ready @ jquery-3.6.0.min_24.1200036.js:2
B @ jquery-3.6.0.min_24.1200036.js:2
AcquisitionsMenu.vue:309 [Vue warn]: injection "permissionsStore" not found. 
  at <App ordermanage=1 orderreceive=1 edifact=0  ... >
warn @ runtime-core.esm-bundler.js:38
inject @ runtime-core.esm-bundler.js:1646
setup @ AcquisitionsMenu.vue:309
callWithErrorHandling @ runtime-core.esm-bundler.js:155
setupStatefulComponent @ runtime-core.esm-bundler.js:7204
setupComponent @ runtime-core.esm-bundler.js:7159
mountComponent @ runtime-core.esm-bundler.js:5508
processComponent @ runtime-core.esm-bundler.js:5483
patch @ runtime-core.esm-bundler.js:5085
render @ runtime-core.esm-bundler.js:6244
render @ runtime-dom.esm-bundler.js:1548
_update @ runtime-dom.esm-bundler.js:591
resolve @ runtime-dom.esm-bundler.js:544
_resolveDef @ runtime-dom.esm-bundler.js:551
connectedCallback @ runtime-dom.esm-bundler.js:478
(anonymous) @ islands.ts:110
step @ index.mjs:34
(anonymous) @ index.mjs:34
asyncGeneratorStep @ index.mjs:34
_next @ index.mjs:34
Promise.then
asyncGeneratorStep @ index.mjs:34
_next @ index.mjs:34
(anonymous) @ index.mjs:34
(anonymous) @ index.mjs:34
(anonymous) @ islands.ts:103
(anonymous) @ islands.ts:103
step @ index.mjs:34
(anonymous) @ index.mjs:34
asyncGeneratorStep @ index.mjs:34
_next @ index.mjs:34
(anonymous) @ index.mjs:34
(anonymous) @ index.mjs:34
requestIdleCallback
hydrate @ islands.ts:87
(anonymous) @ acqui-home.pl:1187
islands.ts:110 [Vue warn]: Unhandled error during execution of setup function 
  at <App ordermanage=1 orderreceive=1 edifact=0  ... >
warn @ runtime-core.esm-bundler.js:38
logError @ runtime-core.esm-bundler.js:212
handleError @ runtime-core.esm-bundler.js:204
callWithErrorHandling @ runtime-core.esm-bundler.js:158
setupStatefulComponent @ runtime-core.esm-bundler.js:7204
setupComponent @ runtime-core.esm-bundler.js:7159
mountComponent @ runtime-core.esm-bundler.js:5508
processComponent @ runtime-core.esm-bundler.js:5483
patch @ runtime-core.esm-bundler.js:5085
render @ runtime-core.esm-bundler.js:6244
render @ runtime-dom.esm-bundler.js:1548
_update @ runtime-dom.esm-bundler.js:591
resolve @ runtime-dom.esm-bundler.js:544
_resolveDef @ runtime-dom.esm-bundler.js:551
connectedCallback @ runtime-dom.esm-bundler.js:478
(anonymous) @ islands.ts:110
step @ index.mjs:34
(anonymous) @ index.mjs:34
asyncGeneratorStep @ index.mjs:34
_next @ index.mjs:34
Promise.then
asyncGeneratorStep @ index.mjs:34
_next @ index.mjs:34
(anonymous) @ index.mjs:34
(anonymous) @ index.mjs:34
(anonymous) @ islands.ts:103
(anonymous) @ islands.ts:103
step @ index.mjs:34
(anonymous) @ index.mjs:34
asyncGeneratorStep @ index.mjs:34
_next @ index.mjs:34
(anonymous) @ index.mjs:34
(anonymous) @ index.mjs:34
requestIdleCallback
hydrate @ islands.ts:87
(anonymous) @ acqui-home.pl:1187
runtime-core.esm-bundler.js:218 Uncaught TypeError: Cannot destructure property 'isUserPermitted' of 'permissionsStore' as it is undefined.
    at setup (AcquisitionsMenu.vue:310:17)
    at callWithErrorHandling (runtime-core.esm-bundler.js:155:1)
    at setupStatefulComponent (runtime-core.esm-bundler.js:7204:1)
    at setupComponent (runtime-core.esm-bundler.js:7159:1)
    at mountComponent (runtime-core.esm-bundler.js:5508:1)
    at processComponent (runtime-core.esm-bundler.js:5483:1)
    at patch (runtime-core.esm-bundler.js:5085:1)
    at Object.render (runtime-core.esm-bundler.js:6244:1)
    at render (runtime-dom.esm-bundler.js:1548:1)
    at HTMLElement._update (runtime-dom.esm-bundler.js:591:1)
setup @ AcquisitionsMenu.vue:310
callWithErrorHandling @ runtime-core.esm-bundler.js:155
setupStatefulComponent @ runtime-core.esm-bundler.js:7204
setupComponent @ runtime-core.esm-bundler.js:7159
mountComponent @ runtime-core.esm-bundler.js:5508
processComponent @ runtime-core.esm-bundler.js:5483
patch @ runtime-core.esm-bundler.js:5085
render @ runtime-core.esm-bundler.js:6244
render @ runtime-dom.esm-bundler.js:1548
_update @ runtime-dom.esm-bundler.js:591
resolve @ runtime-dom.esm-bundler.js:544
_resolveDef @ runtime-dom.esm-bundler.js:551
connectedCallback @ runtime-dom.esm-bundler.js:478
(anonymous) @ islands.ts:110
step @ index.mjs:34
(anonymous) @ index.mjs:34
asyncGeneratorStep @ index.mjs:34
_next @ index.mjs:34
Promise.then
asyncGeneratorStep @ index.mjs:34
_next @ index.mjs:34
(anonymous) @ index.mjs:34
(anonymous) @ index.mjs:34
(anonymous) @ islands.ts:103
(anonymous) @ islands.ts:103
step @ index.mjs:34
(anonymous) @ index.mjs:34
asyncGeneratorStep @ index.mjs:34
_next @ index.mjs:34
(anonymous) @ index.mjs:34
(anonymous) @ index.mjs:34
requestIdleCallback
hydrate @ islands.ts:87
(anonymous) @ acqui-home.pl:1187

And also:
Sticky toolbar
    1) Should stick on scroll


  0 passing (3s)
  1 failing

  1) Sticky toolbar
       Should stick on scroll:
     TypeError: The following error originated from your application code, not from Cypress.

  > Cannot destructure property 'isUserPermitted' of 'permissionsStore' as it is undefined.

When Cypress detects uncaught errors originating from your application it will automatically fail the current test.

This behavior is configurable, and you can choose to turn this off by listening to the `uncaught:exception` event.

https://on.cypress.io/uncaught-exception-from-application
      at setup (http://localhost:8081/intranet-tmpl/prog/js/vue/dist/acquisitions-menu.09f785f53cd6ce2fae1b.esm.js:99:17)
      at callWithErrorHandling (http://localhost:8081/intranet-tmpl/prog/js/vue/dist/islands.esm_24.1200036.js:3673:22)
      at setupStatefulComponent (http://localhost:8081/intranet-tmpl/prog/js/vue/dist/islands.esm_24.1200036.js:10706:29)
      at setupComponent (http://localhost:8081/intranet-tmpl/prog/js/vue/dist/islands.esm_24.1200036.js:10661:11)
      at mountComponent (http://localhost:8081/intranet-tmpl/prog/js/vue/dist/islands.esm_24.1200036.js:9012:13)
      at processComponent (http://localhost:8081/intranet-tmpl/prog/js/vue/dist/islands.esm_24.1200036.js:8987:17)
      at patch (http://localhost:8081/intranet-tmpl/prog/js/vue/dist/islands.esm_24.1200036.js:8589:21)
      at Object.render (http://localhost:8081/intranet-tmpl/prog/js/vue/dist/islands.esm_24.1200036.js:9748:13)
      at render (http://localhost:8081/intranet-tmpl/prog/js/vue/dist/islands.esm_24.1200036.js:13085:22)
      at HTMLElement._update (http://localhost:8081/intranet-tmpl/prog/js/vue/dist/islands.esm_24.1200036.js:12128:9)

I am unsure whether I applied something wrong, or whether this is a real failure?

Setting this to FQA, please reset to 'Signed off' if I made a mistake somewhere:
- Applied this bug and all dependencies onto current upstream/main
- reset_all as suggested
- yarn build
- restart_all
- Manually tested stickyness (which works and is not the reason for FQA)
  - Saw the above mentioned errors + warnings in the console acqui-home.pl
- Ran the cypress spec, ran into above failure
Comment 16 Jonathan Druart 2025-05-05 08:43:46 UTC
It works for me, however can we make it match the patron toolbar?

https://snipboard.io/VBxNCj.jpg
vs
https://snipboard.io/v2JSHZ.jpg

1. Missing "save" icon
2. Cancel misaligned and not green (appears grey)
Comment 17 Matt Blenkinsop 2025-05-06 10:35:07 UTC
Created attachment 181965 [details] [review]
Bug 38899: (QA follow-up) Match buttons to patron toolbar
Comment 18 Lucas Gass (lukeg) 2025-05-08 19:11:50 UTC
Looks good, works good. Do we need to tidy vue and ts files?


 FAIL	koha-tmpl/intranet-tmpl/prog/js/vue/components/ButtonSubmit.vue
   FAIL	  tidiness
		File is not tidy, please run `perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/js/vue/components/ButtonSubmit.vue`


 FAIL	koha-tmpl/intranet-tmpl/prog/js/vue/modules/acquisitions.ts
   FAIL	  tidiness
		File is not tidy, please run `perl misc/devel/tidy.pl koha-tmpl/intranet-tmpl/prog/js/vue/modules/acquisitions.ts`
Comment 19 Matt Blenkinsop 2025-05-09 07:28:27 UTC
Created attachment 182151 [details] [review]
Bug 38899: (QA follow-up) Match buttons to patron toolbar
Comment 20 Matt Blenkinsop 2025-05-09 07:29:15 UTC
Tidied and squashed to last commit - not sure why the pre-commit hook tidy didn't catch those but QA tests are passing now
Comment 21 Jonathan Druart 2025-05-12 07:43:29 UTC
Still noticing a tiny difference between the 2 versions of the toolbar (comparing vendor edit and patron edit): I think it's coming from the use of <font-awesome-icon> vs <i>.

Actually the Vue version is better (the vertical alignment) now IMO :)
Comment 22 Jonathan Druart 2025-05-12 07:52:51 UTC
There is something wrong.
Cypress tests are now failing, because you renamed #vendor_add with #add_vendor.

But renaming in the test is not enough, as I think it's not consistent:
* the naming is not "add_vendor" vs "vendors_list", etc.
* the block on which the id is is not consistent:

main
  #vendors_list
    #toolbar
    .page-section
      the content
vs

main
  h1
  #add_vendor
    #toolbar
    the content
Comment 23 Jonathan Druart 2025-05-12 07:58:32 UTC
Created attachment 182249 [details] [review]
Bug 38899: Fix cypress tests
Comment 24 Jonathan Druart 2025-05-12 07:58:35 UTC
Created attachment 182250 [details] [review]
Bug 38899: Move the toolbar out of the form
Comment 25 Jonathan Druart 2025-05-12 08:00:59 UTC
Well, the tests are still failing.

AssertionError: Timed out retrying after 10000ms: Expected to find content: 'Submit' within the element: <form#add_vendor> but never did.
Comment 26 Matt Blenkinsop 2025-05-12 10:43:38 UTC
Created attachment 182255 [details] [review]
Bug 38899: Improve HTML consistency
Comment 27 Jonathan Druart 2025-05-12 13:32:16 UTC
(In reply to Matt Blenkinsop from comment #26)
> Created attachment 182255 [details] [review] [review]
> Bug 38899: Improve HTML consistency

No, this is wrong. We now have 2 "main".

I think we should have:

main
  #vendors_add
    #toolbar
    .page-section
      form
Comment 28 Jonathan Druart 2025-05-14 14:04:46 UTC
Created attachment 182449 [details] [review]
Bug 38899: Remove code duplication
Comment 29 Katrin Fischer 2025-05-16 15:07:46 UTC
Where are we at here? I think this is still needed for 25.05?
Comment 30 Matt Blenkinsop 2025-05-19 10:10:43 UTC
Created attachment 182580 [details] [review]
Bug 38899: Restore id attribute to the form
Comment 31 Matt Blenkinsop 2025-05-19 10:11:53 UTC
This is the closest I can get with the stickyness still working - if I replace the template tag with a div, the toolbar just scrolls up with the form
Comment 32 Martin Renvoize (ashimema) 2025-05-19 18:17:20 UTC
I'm still seeing failing cypress tests for this I'm afraid.  Vendors in particular.
Comment 33 Matt Blenkinsop 2025-05-20 08:39:42 UTC
Created attachment 182646 [details] [review]
Bug 38899: Fix cypress tests
Comment 34 Katrin Fischer 2025-05-26 13:36:31 UTC
Let's aim for this one early next cycle?
Comment 35 David Nind 2025-05-26 20:00:21 UTC
I've changed this from an enhancement to a normal bug.

While it is an enhancement to Vue, it restores something that existed before, for vendors in the acquisitions module.
Comment 36 Matt Blenkinsop 2025-06-05 09:12:48 UTC
Created attachment 182973 [details] [review]
Bug 38899: Add a sticky toolbar

This patch adds a sticky toolbar to the vendors form

Test plan:
N.B: This patchset will not work on a sandbox

1) Navigate to Acquisitions and click New vendor
2) The toolbar should appear at the top of the page underneath Add vendor
3) Scroll down, the toolbar should stick and be given the 'floating' class
4) Scroll back up and the toolbar should unstick and lose the 'floating' class

Signed-off-by: David Nind <david@davidnind.com>
Comment 37 Matt Blenkinsop 2025-06-05 09:12:52 UTC
Created attachment 182974 [details] [review]
Bug 38899: Add a cypress test

Signed-off-by: David Nind <david@davidnind.com>
Comment 38 Matt Blenkinsop 2025-06-05 09:12:56 UTC
Created attachment 182975 [details] [review]
Bug 38899: (QA follow-up) Match buttons to patron toolbar
Comment 39 Matt Blenkinsop 2025-06-05 09:13:06 UTC
Created attachment 182976 [details] [review]
Bug 38899: Move the toolbar out of the form
Comment 40 Matt Blenkinsop 2025-06-05 09:13:12 UTC
Created attachment 182977 [details] [review]
Bug 38899: Improve HTML consistency
Comment 41 Matt Blenkinsop 2025-06-05 09:13:16 UTC
Created attachment 182978 [details] [review]
Bug 38899: Remove code duplication
Comment 42 Matt Blenkinsop 2025-06-05 09:13:20 UTC
Created attachment 182979 [details] [review]
Bug 38899: Restore id attribute to the form
Comment 43 Matt Blenkinsop 2025-06-05 09:13:25 UTC
Created attachment 182980 [details] [review]
Bug 38899: Fix cypress tests
Comment 44 Jonathan Druart 2025-06-10 09:31:43 UTC
Created attachment 183114 [details] [review]
Bug 38899: Add a sticky toolbar

This patch adds a sticky toolbar to the vendors form

Test plan:
N.B: This patchset will not work on a sandbox

1) Navigate to Acquisitions and click New vendor
2) The toolbar should appear at the top of the page underneath Add vendor
3) Scroll down, the toolbar should stick and be given the 'floating' class
4) Scroll back up and the toolbar should unstick and lose the 'floating' class

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 45 Jonathan Druart 2025-06-10 09:31:45 UTC
Created attachment 183115 [details] [review]
Bug 38899: Add a cypress test

Signed-off-by: David Nind <david@davidnind.com>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 46 Jonathan Druart 2025-06-10 09:31:48 UTC
Created attachment 183116 [details] [review]
Bug 38899: (QA follow-up) Match buttons to patron toolbar

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 47 Jonathan Druart 2025-06-10 09:31:50 UTC
Created attachment 183117 [details] [review]
Bug 38899: Move the toolbar out of the form

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 48 Jonathan Druart 2025-06-10 09:31:53 UTC
Created attachment 183118 [details] [review]
Bug 38899: Improve HTML consistency

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 49 Jonathan Druart 2025-06-10 09:31:55 UTC
Created attachment 183119 [details] [review]
Bug 38899: Remove code duplication

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 50 Jonathan Druart 2025-06-10 09:31:57 UTC
Created attachment 183120 [details] [review]
Bug 38899: Restore id attribute to the form

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 51 Jonathan Druart 2025-06-10 09:32:00 UTC
Created attachment 183121 [details] [review]
Bug 38899: Fix cypress tests

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>
Comment 52 Lucas Gass (lukeg) 2025-06-11 20:07:51 UTC
Nice work everyone!

Pushed to main for 25.11