Bug 33169 - Improve vue breadcrumbs and left-hand menu
Summary: Improve vue breadcrumbs and left-hand menu
Status: Pushed to oldstable
Alias: None
Product: Koha
Classification: Unclassified
Component: Staff interface (show other bugs)
Version: unspecified
Hardware: All All
: P5 - low enhancement (vote)
Assignee: Agustín Moyano
QA Contact: Testopia
URL:
Keywords: rel_23_05_candidate
Depends on: 32932 32991
Blocks: 33606 34417 34735 35919 32607 34418 34691
  Show dependency treegraph
 
Reported: 2023-03-08 15:03 UTC by Agustín Moyano
Modified: 2024-01-26 13:50 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
23.11.00,23.05.05,22.11.12


Attachments
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition (46.04 KB, patch)
2023-03-08 15:06 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 33169: (follow-up) Cleanup and tidy a little bit (27.31 KB, patch)
2023-03-09 23:03 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 33169: (follow-up) Cleanup and tidy a little bit (27.41 KB, patch)
2023-03-10 16:10 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition (53.59 KB, patch)
2023-03-10 20:09 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition (53.89 KB, patch)
2023-04-14 22:46 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition (53.97 KB, patch)
2023-04-17 09:44 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition (53.96 KB, patch)
2023-05-22 13:44 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition (53.88 KB, patch)
2023-05-30 12:47 UTC, Agustín Moyano
Details | Diff | Splinter Review
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition (53.95 KB, patch)
2023-06-06 19:32 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition (53.97 KB, patch)
2023-06-21 19:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33169: Add support for FA styles (6.28 KB, patch)
2023-06-21 19:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33169: (QA follow-up) Tidy Vue-related files (25.73 KB, patch)
2023-06-21 19:25 UTC, Tomás Cohen Arazi
Details | Diff | Splinter Review
Bug 33169: Allow breadcrumbs to accept dynamic values (19.99 KB, patch)
2023-07-25 17:21 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
Bug 33169: DO NOT PUSH Allow breadcrumbs to accept dynamic values (20.01 KB, patch)
2023-07-25 17:23 UTC, Matt Blenkinsop
Details | Diff | Splinter Review
[23.05] Bug 33169: (QA follow-up) Fix some icons for lower FA version (16.44 KB, patch)
2023-10-25 11:51 UTC, Katrin Fischer
Details | Diff | Splinter Review
[23.05 without DNP-Patch] Bug 33169: (QA follow-up) Fix some icons for lower FA version (1.63 KB, patch)
2023-10-25 15:02 UTC, Katrin Fischer
Details | Diff | Splinter Review
[23.05 without DNP-Patch] Bug 33169: (QA follow-up) Fix some icons for lower FA version (2.07 KB, patch)
2023-10-25 15:04 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Agustín Moyano 2023-03-08 15:03:48 UTC
Jonathan made a great vue component in bug 32030 to build the breadcrumbs from vue-routes, but I believe it can be improved.

With this implementation, I believe it will be a little cleaner to define the routes, and besides that, we can use the same definition to build the left-hand side menu.

Now if you want to add a new option, there is only one place to modify (the routes definition) and everything else will be built around it.
Comment 1 Agustín Moyano 2023-03-08 15:06:22 UTC
Created attachment 147962 [details] [review]
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition
Comment 2 Jonathan Druart 2023-03-08 15:27:17 UTC
1. You forgot the translatability ;)

2. There are 2 warnings in the console

 Vue received a Component which was made a reactive object. This can lead to unnecessary performance overhead, and should be avoided by marking the component with `markRaw` or using `shallowRef` instead of `ref`. 

3. The last item does not have the "current" and so is not styled correctly (green instead of grey)

4. The disabled ("Local" for instance) is also styled differently

5. This will conflict with bug 32932

6. vue/stores/navigation.js should be tidied :)
Comment 3 Agustín Moyano 2023-03-08 15:45:47 UTC
(In reply to Jonathan Druart from comment #2)
> 1. You forgot the translatability ;)
> 
> 2. There are 2 warnings in the console
> 
>  Vue received a Component which was made a reactive object. This can lead to
> unnecessary performance overhead, and should be avoided by marking the
> component with `markRaw` or using `shallowRef` instead of `ref`. 
> 
> 3. The last item does not have the "current" and so is not styled correctly
> (green instead of grey)
> 
> 4. The disabled ("Local" for instance) is also styled differently
> 
> 5. This will conflict with bug 32932
> 
> 6. vue/stores/navigation.js should be tidied :)

Thank you Jonathan for the feedback!
Comment 4 Agustín Moyano 2023-03-09 23:03:22 UTC
Created attachment 148044 [details] [review]
Bug 33169: (follow-up) Cleanup and tidy a little bit
Comment 5 Agustín Moyano 2023-03-10 16:10:19 UTC
Created attachment 148081 [details] [review]
Bug 33169: (follow-up) Cleanup and tidy a little bit
Comment 6 Agustín Moyano 2023-03-10 20:09:21 UTC
Created attachment 148097 [details] [review]
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition
Comment 7 Pedro Amorim 2023-03-25 17:24:55 UTC Comment hidden (obsolete)
Comment 8 Agustín Moyano 2023-04-14 22:46:52 UTC
Created attachment 149710 [details] [review]
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition
Comment 9 David Nind 2023-04-16 20:13:25 UTC
Changed assignee 8-).
Comment 10 Matt Blenkinsop 2023-04-17 09:44:42 UTC
Created attachment 149744 [details] [review]
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 11 Jonathan Druart 2023-05-10 08:10:18 UTC
There is something broken.

Uncaught (in promise) Error: Missing required param "license_id"

To recreate:
1. Create a license
2. Go to /cgi-bin/koha/erm/licenses
3. Click on a license's name
Comment 12 Agustín Moyano 2023-05-22 13:44:22 UTC
Created attachment 151530 [details] [review]
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 13 Agustín Moyano 2023-05-22 13:45:13 UTC
(In reply to Jonathan Druart from comment #11)
> There is something broken.
> 
> Uncaught (in promise) Error: Missing required param "license_id"
> 
> To recreate:
> 1. Create a license
> 2. Go to /cgi-bin/koha/erm/licenses
> 3. Click on a license's name

Could no reproduce this bug... could you please try again?
Comment 14 Jonathan Druart 2023-05-23 08:47:50 UTC
(In reply to Agustín Moyano from comment #13)
> (In reply to Jonathan Druart from comment #11)
> > There is something broken.
> > 
> > Uncaught (in promise) Error: Missing required param "license_id"
> > 
> > To recreate:
> > 1. Create a license
> > 2. Go to /cgi-bin/koha/erm/licenses
> > 3. Click on a license's name
> 
> Could no reproduce this bug... could you please try again?

Tried again and still the problem.

yes|git bz apply 33169
yarn js:build
Create a license
Go to /cgi-bin/koha/erm/licenses
Open the console
Click on a license's name

Uncaught (in promise) Error: Missing required param "license_id"
Comment 15 Agustín Moyano 2023-05-30 12:47:53 UTC
Created attachment 151797 [details] [review]
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Comment 16 Agustín Moyano 2023-05-30 12:49:18 UTC
Hi Jonathan, please try once more.

Thanks
Comment 17 Tomás Cohen Arazi 2023-06-06 19:32:29 UTC
Created attachment 152068 [details] [review]
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 18 Tomás Cohen Arazi 2023-06-06 19:36:54 UTC
Not changing the status as I aint no Vue expert. But I tested things work and no warnings/errors in the console.

The only thing I don't like but is not this patch responsibility, is the fact we display 'Show Thing' in the breadcrumb... I think we should be more consistent (we are not 100% consistent on this, I know but...).

Right now we display

Home > E-resource management > Licenses > Show license

it should be:

Home > E-resource management > Licenses > A licence name

like we have in other places e.g

Home > Tools > Patron lists > A list name

I mention this in case it fits this change.
Comment 19 Jonathan Druart 2023-06-07 07:37:46 UTC
(In reply to Tomás Cohen Arazi from comment #18)
> Not changing the status as I aint no Vue expert. But I tested things work
> and no warnings/errors in the console.
> 
> The only thing I don't like but is not this patch responsibility, is the
> fact we display 'Show Thing' in the breadcrumb... I think we should be more
> consistent (we are not 100% consistent on this, I know but...).
> 
> Right now we display
> 
> Home > E-resource management > Licenses > Show license
> 
> it should be:
> 
> Home > E-resource management > Licenses > A licence name
> 
> like we have in other places e.g
> 
> Home > Tools > Patron lists > A list name
> 
> I mention this in case it fits this change.

It is something I didn't manage to do with the current code (I thought there was actually a FIXME in the code but there is none).
Comment 20 Tomás Cohen Arazi 2023-06-21 19:25:12 UTC
Created attachment 152539 [details] [review]
Bug 33169: Build breadcrumbs and left-hand side menu from routes definition

Signed-off-by: Matt Blenkinsop <matt.blenkinsop@ptfs-europe.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 21 Tomás Cohen Arazi 2023-06-21 19:25:15 UTC
Created attachment 152540 [details] [review]
Bug 33169: Add support for FA styles

Font awesome now provides a different approach to icon styles so you can
specify things like:

```
<i class="fa fa-camera-retro"></i>
```

but also

```
<i class="fa-solid fa-camera-retro"></i>
```

The original implementation had a hardcoded `fa` and having an extra
parameter for the *style* seemed overkill, as we would need to handle
even a list of styles like on this example:

```
<i class="fa-sharp fa-solid fa-camera-retro"></i>
```

So I chose to just explicitly require the full FA icon class and pass it
thru. With no manipulation on the Vue side.

This patch does that, and adjusts the 'style' for some icons, as
introduced by the FA v6 patchset.

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 22 Tomás Cohen Arazi 2023-06-21 19:25:19 UTC
Created attachment 152541 [details] [review]
Bug 33169: (QA follow-up) Tidy Vue-related files

Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>
Comment 23 Agustín Moyano 2023-06-21 23:06:34 UTC
(In reply to Tomás Cohen Arazi from comment #22)
> Created attachment 152541 [details] [review] [review]
> Bug 33169: (QA follow-up) Tidy Vue-related files
> 
> Signed-off-by: Tomas Cohen Arazi <tomascohen@theke.io>

Thanks Tom for the follow-ups, I'll mark this as PQA
Comment 24 Agustín Moyano 2023-06-21 23:08:55 UTC
(In reply to Jonathan Druart from comment #19)
> (In reply to Tomás Cohen Arazi from comment #18)
> > Not changing the status as I aint no Vue expert. But I tested things work
> > and no warnings/errors in the console.
> > 
> > The only thing I don't like but is not this patch responsibility, is the
> > fact we display 'Show Thing' in the breadcrumb... I think we should be more
> > consistent (we are not 100% consistent on this, I know but...).
> > 
> > Right now we display
> > 
> > Home > E-resource management > Licenses > Show license
> > 
> > it should be:
> > 
> > Home > E-resource management > Licenses > A licence name
> > 
> > like we have in other places e.g
> > 
> > Home > Tools > Patron lists > A list name
> > 
> > I mention this in case it fits this change.
> 
> It is something I didn't manage to do with the current code (I thought there
> was actually a FIXME in the code but there is none).

Maybe later I can add a method to the navigation store to be able to update the last part of the breadcrumb

I'll create a new bug for it.
Comment 25 Tomás Cohen Arazi 2023-06-23 13:01:45 UTC
Pushed to master for 23.11.

Nice work everyone, thanks!
Comment 26 Jonathan Druart 2023-07-04 15:10:07 UTC
Independent QA missing here...
Comment 27 Jonathan Druart 2023-07-06 11:03:36 UTC
1. Existing components/Breadcrumb.vue has not been removed. But please don't remove until the next point is not fixed.

2. I am trying to adjust the code from bug 30708 to use this, but I am struggling. I don't know if it's me or this implementation that is not flexible enough.

I am trying to have, in the breadcrumbs:
/preservation/trains/3
=> Preservation > Trains > Show train

/preservation/trains/3/items/add
=> Preservation > Trains > Add item to train

Ideally we will need to display the name of the current resource, but that's another topic. I don't think it's feasible for now (discussed already in comment 18 and comment 19).

With my try on https://gitlab.com/joubu/Koha/-/commits/preservation_module-wip-33169 I am getting:

/preservation/trains/3
=> Preservation > Trains > Show train (OK)
/preservation/trains/3/items/add
=> Preservation > Trains > Show train > > Add item to train (KO there are 2 '>')
Additionally "Show train" is /preservation/trains/:train_id
Comment 28 Jonathan Druart 2023-07-06 14:50:46 UTC
Not sure this is related to this patch, but I am getting weird warning that seems coming from here

[Vue Router warn]: Discarded invalid param(s) "license_id" when navigating. See https://github.com/vuejs/router/blob/main/packages/router/CHANGELOG.md#414-2022-08-22 for more details.

It happens, inconsistently, when going to a FormAdd view (License or Agreement)
http://kohadev-intra.mydnsname.org:8081/cgi-bin/koha/erm/licenses/add

When creating a new agreement the error is about agreement_id ofc.
Comment 29 Martin Renvoize 2023-07-17 13:48:42 UTC
I think this should be backported once the above issues are resolved.. it's likely to be needed for subsequent backports around erm and other vue based modules.
Comment 30 Jonathan Druart 2023-07-20 09:09:42 UTC
Ping?

The problems highlighted in the previous comments are blocker for future Vue developments (already in testing stage!).
Comment 31 Jonathan Druart 2023-07-25 06:28:58 UTC
From IRC

Joubu> tcohen[m]: any news from AgustinMoyano[m] about bug 33169?                                                                                                                                                    
huginn> ^C04Bug https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=33169 enhancement, P5 - low, ---, agustinmoyano, Pushed to master , Improve vue breadcrumbs and left-hand menu^C
tcohen[m]> I wouldn't count on him taking on that
tcohen[m]> ashimema asked me, and I told him I haven't heard from him 
ashimema> It's bad we now how two ways of doing almost the same thing
tcohen[m]> ashimema: we have two ways because the previous wasn't removed, but it is unused, right?
tcohen[m]> the missing piece here would be addressing Joubu's problem reports, right?
Joubu> tcohen[m]: then who? If nobody we should revert then...
Joubu> we complexified the code bringing less flexibility...
tcohen[m]> it didn't look complex to me, and had Matt's signature
ashimema> I'm not actually sure
tcohen[m]> I can revert, but we need to make sure this is not what we are aiming for 
ashimema> I think both are used
ashimema> maybe I'm wrong
tcohen[m]> the patch replaced the 'old ones'
tcohen[m]> cd koha-tmpl/intranet-tmpl/prog/js/vue
tcohen[m]> git grep Breadcrumb
tcohen[m]> I'd like to hear from Matthew Blenkinsop.
tcohen[m]> he saw some value on this on the other bug 
tcohen[m]> I'm not attached to it
Joubu> At least I would like to know if what I want to do is possible or if we need more code
Joubu> I've tried several things but didn't manage with the current code, and I don't understand the code to fix it
tcohen[m]> so you tried but didn't manage to fix it
tcohen[m]> ok
Joubu> if nobody understand it, apart from AgustinMoyano[m] that is not going to spend more time on it, then it's hard to keep...
Joubu> ofc I've tried
tcohen[m]> if nobody understands it
tcohen[m]> it doesn't matter if he fixes it, right?
Joubu> QA step is there to make sure not only 1 person understand the code, right? :D
tcohen[m]> the code in both is almost identical
tcohen[m]> well no
tcohen[m]> there's the 'NavigationItem' abstraction

Joubu> there is almost nothing identical between the 2 approaches                                                                                                                                                    
Joubu> in "mine" there is a build_breadcrumb function for each item
Joubu> in the "new" one, there are some flag defined in the route definition (is_navigation_item, is_end_node, etc.) then the logic is in navigation.js
Joubu> store/navigation.js
Joubu> that is where the logic is*
Comment 32 Jonathan Druart 2023-07-25 06:32:36 UTC
I really would like to see this problem fixed, and it is preferable to fix this new implementation as it is better in the long term (less code to add and maintain in router files).
However if nobody is able to provide a fix then we have a problem: we have code that we don't have under control. Or, maybe we don't even need a fix, and the current code already allows it? But how then?
Comment 33 Matt Blenkinsop 2023-07-25 17:21:24 UTC
Created attachment 153889 [details] [review]
Bug 33169: Allow breadcrumbs to accept dynamic values

This patch is an example of allowing a breadcrumb to accept a dynamic value such as an agreement or license name.
Comment 34 Matt Blenkinsop 2023-07-25 17:23:58 UTC
Created attachment 153890 [details] [review]
Bug 33169: DO NOT PUSH  Allow breadcrumbs to accept dynamic values

This patch is an example of allowing a breadcrumb to accept a dynamic value such as an agreement or license name.
Comment 35 Matt Blenkinsop 2023-07-25 17:29:20 UTC
The patch above is an example of an implementation for this by passing a parameter into the breadcrumbs from a particular component.

Currently I've only set it up for Agreements and Licenses for discussion but the implementation is simple to repeat.

I've also included two dummy routes to tackle Joubu's issue in comment 27 that would need removing once this example is reviewed. These have been added into Agreements as a substitute for the Preservation module

Test plan
1) Apply and yarn js:build
2) Create an agreement
3) Click on the link to view the agreement
4) The breadcrumb should contain your agreement's name
5) Navigate to /cgi-bin/koha/erm/agreements/1/items/add/
6) Breadcrumb should read "Agreements" > "Your agreement name" > "Add item to train"
7) Navigate to /cgi-bin/koha/erm/agreements/1/items/add/1
8) Breadcrumb should read "Agreements" > "Your agreement name" > "Add items to train"
Comment 36 Fridolin Somers 2023-07-25 20:13:34 UTC
rel_23_05_candidate means we want backport on 23.05.x ?
Patches does not apply easy because of Bug 32910 not in 23.05
Comment 37 Martin Renvoize 2023-07-26 09:42:51 UTC
Isn't it just a case of skipping the 'Add support for FA styles' patch?
Comment 38 Jonathan Druart 2023-07-26 11:17:05 UTC
(In reply to Matt Blenkinsop from comment #35)
> The patch above is an example of an implementation for this by passing a
> parameter into the breadcrumbs from a particular component.
> 
> Currently I've only set it up for Agreements and Licenses for discussion but
> the implementation is simple to repeat.
> 
> I've also included two dummy routes to tackle Joubu's issue in comment 27
> that would need removing once this example is reviewed. These have been
> added into Agreements as a substitute for the Preservation module
> 
> Test plan
> 1) Apply and yarn js:build
> 2) Create an agreement
> 3) Click on the link to view the agreement
> 4) The breadcrumb should contain your agreement's name
> 5) Navigate to /cgi-bin/koha/erm/agreements/1/items/add/
> 6) Breadcrumb should read "Agreements" > "Your agreement name" > "Add item
> to train"
> 7) Navigate to /cgi-bin/koha/erm/agreements/1/items/add/1
> 8) Breadcrumb should read "Agreements" > "Your agreement name" > "Add items
> to train"

Thanks a lot Matt, I have opened 2 bug reports, I will split your patch. See bug 34417 and 34418. See you there ;)
Comment 39 Agustín Moyano 2023-07-26 20:34:22 UTC
(In reply to Jonathan Druart from comment #38)
> (In reply to Matt Blenkinsop from comment #35)
> > The patch above is an example of an implementation for this by passing a
> > parameter into the breadcrumbs from a particular component.
> > 
> > Currently I've only set it up for Agreements and Licenses for discussion but
> > the implementation is simple to repeat.
> > 
> > I've also included two dummy routes to tackle Joubu's issue in comment 27
> > that would need removing once this example is reviewed. These have been
> > added into Agreements as a substitute for the Preservation module
> > 
> > Test plan
> > 1) Apply and yarn js:build
> > 2) Create an agreement
> > 3) Click on the link to view the agreement
> > 4) The breadcrumb should contain your agreement's name
> > 5) Navigate to /cgi-bin/koha/erm/agreements/1/items/add/
> > 6) Breadcrumb should read "Agreements" > "Your agreement name" > "Add item
> > to train"
> > 7) Navigate to /cgi-bin/koha/erm/agreements/1/items/add/1
> > 8) Breadcrumb should read "Agreements" > "Your agreement name" > "Add items
> > to train"
> 
> Thanks a lot Matt, I have opened 2 bug reports, I will split your patch. See
> bug 34417 and 34418. See you there ;)

I did not see Matt's reply before... I uploaded a patch to fix this problem in bug 34425

Could you and Matt check it out, please?

Thanks
Comment 40 Katrin Fischer 2023-10-14 12:29:12 UTC
(In reply to Fridolin Somers from comment #36)
> rel_23_05_candidate means we want backport on 23.05.x ?
> Patches does not apply easy because of Bug 32910 not in 23.05

Any update here?
Comment 41 Katrin Fischer 2023-10-25 11:51:49 UTC
Created attachment 157785 [details] [review]
[23.05] Bug 33169: (QA follow-up) Fix some icons for lower FA version

In the navigation some icons were missing, because they had changed
names between versions, this restores them. Fixed icons concerned
the navigation items:

* Local
* Local > Titles
* EBSCO > Titles
Comment 42 Katrin Fischer 2023-10-25 15:02:22 UTC
Created attachment 157806 [details] [review]
[23.05 without DNP-Patch] Bug 33169: (QA follow-up) Fix some icons for lower FA version

In the navigation some icons were missing, because they had changed
names between versions, this restores them. Fixed icons concerned
the navigation items:

* Local
* Local > Titles
* EBSCO > Titles
Comment 43 Katrin Fischer 2023-10-25 15:04:05 UTC
Created attachment 157807 [details] [review]
[23.05 without DNP-Patch] Bug 33169: (QA follow-up) Fix some icons for lower FA version

In the navigation some icons were missing, because they had changed
names between versions, this restores them. Fixed icons concerned
the navigation items:

* Local
* Local > Titles
* EBSCO > Titles
Comment 44 Katrin Fischer 2023-10-25 15:04:50 UTC
The follow-up did not apply if the DO NOT PUSH patch was not included. I made a patch that works on top of a branch without that patch too.
Comment 45 Fridolin Somers 2023-11-02 21:26:57 UTC
I think I get it.

I've picked from branch https://gitlab.com/joubu/Koha/-/commits/bug_33606-23.05 :

7202d42b2d Bug 33169: (QA follow-up) Tidy Vue-related files
6079d70e2d Bug 33169: Add support for FA styles
31f48a6d19 Bug 33169: Build breadcrumbs and left-hand side menu from routes definition

Plus the last patch attached here :

102b21d2ae Bug 33169: (QA follow-up) Fix some icons for lower FA version

Looks good with Bug 33606

Pushed to 23.05.x for 23.05.05
Comment 46 Katrin Fischer 2023-11-02 23:15:40 UTC
(In reply to Fridolin Somers from comment #45)
> I think I get it.
> 
> I've picked from branch
> https://gitlab.com/joubu/Koha/-/commits/bug_33606-23.05 :
> 
> 7202d42b2d Bug 33169: (QA follow-up) Tidy Vue-related files
> 6079d70e2d Bug 33169: Add support for FA styles
> 31f48a6d19 Bug 33169: Build breadcrumbs and left-hand side menu from routes
> definition
> 
> Plus the last patch attached here :
> 
> 102b21d2ae Bug 33169: (QA follow-up) Fix some icons for lower FA version
> 
> Looks good with Bug 33606
> 
> Pushed to 23.05.x for 23.05.05

Hi Frido, please pick the last patch from here too - it fixes 2 broken icons in the ERM navigation (3 menu items appear without an icon, if you have all features active)
Comment 47 Katrin Fischer 2023-11-02 23:16:36 UTC
(In reply to Katrin Fischer from comment #46)
> (In reply to Fridolin Somers from comment #45)
> > I think I get it.
> > 
> > I've picked from branch
> > https://gitlab.com/joubu/Koha/-/commits/bug_33606-23.05 :
> > 
> > 7202d42b2d Bug 33169: (QA follow-up) Tidy Vue-related files
> > 6079d70e2d Bug 33169: Add support for FA styles
> > 31f48a6d19 Bug 33169: Build breadcrumbs and left-hand side menu from routes
> > definition
> > 
> > Plus the last patch attached here :
> > 
> > 102b21d2ae Bug 33169: (QA follow-up) Fix some icons for lower FA version
> > 
> > Looks good with Bug 33606
> > 
> > Pushed to 23.05.x for 23.05.05
> 
> Hi Frido, please pick the last patch from here too - it fixes 2 broken icons
> in the ERM navigation (3 menu items appear without an icon, if you have all
> features active)

Clearly to late for writing comment - you got them all :) Thanks!
Comment 48 Fridolin Somers 2023-11-03 08:26:42 UTC
(In reply to Katrin Fischer from comment #47)
> 
> Clearly to late for writing comment - you got them all :) Thanks!

Ouf ;)
Comment 49 Matt Blenkinsop 2023-11-13 15:11:11 UTC
Nice work everyone!

Pushed to oldstable for 22.11.x