OBPIH-5273 Dashboard is underlined in menu on some pages #3770

alannadolny · 2023-01-10T20:21:45Z

No description provided.

drodzewicz · 2023-01-11T14:12:00Z

grails-app/views/dashboard/megamenu.gsp

-      const path = window.location.pathname
-      const menuConfigValues = $(".menu-config-value").toArray();
-      const matchingSection = menuConfigValues.find(it => it.value.includes(path))
+    function getHrefs(value) {


this function seems a little too complicated which makes it very hard to read and understand what is going on.
I think that we should make it more explicit and straight forward.
My proposition on how to deal with certain pages that are not underlined in the menu for GSP pages is to add a script in eg. show.gsp for stocklists

<script type="text/javascript"> $(document).ready(function () { const matchingMenuSection = $("#requisitionTemplate') const matchingMenuSectionCollapsable = $("#requisitionTemplate-collapsed").get(0); if (matchingMenuSection) matchingMenuSection.classList.add('active-section'); if (matchingMenuSectionCollapsable) matchingMenuSectionCollapsable.classList.add('active-section'); }) </script>

that way when this show page is going to be rendered the script is going to add an active class to the appropriate section

of course you would have to implement this script in all of the pages that require it but I don't think this will be a big problem since it's a simple logic and I would prefer some duplicate chunks of code instead of a complicated hard to read function

drodzewicz · 2023-01-11T14:13:18Z

grails-app/conf/Config.groovy

@@ -1340,7 +1340,7 @@ openboxes {
                        [label: "inventory.import.label", defaultLabel: "Import Inventory", href: "/${appName}/batch/importData?type=inventory&execution=e1s1"],
                        [label: "inventory.createStockTransfer.label", defaultLabel: "Create Stock Transfer", requiredActivitiesAll: ActivityCode.binTrackingList(), href: "/${appName}/stockTransfer/create"],
                        [label: "inventory.listStockTransfers.label", defaultLabel: "List Stock Transfers", requiredActivitiesAll: ActivityCode.binTrackingList(), href: "/${appName}/stockTransfer/list"],
-                        [label: "inventory.createReplenishment.label", defaultLabel: "Create Replenishment", requiredActivitiesAll: ActivityCode.binTrackingList(), href: "/${appName}/replenishment/index"]
+                        [label: "inventory.createReplenishment.label", defaultLabel: "Create Replenishment", requiredActivitiesAll: ActivityCode.binTrackingList(), href: "/${appName}/replenishment/create"]
                    ]


[label: "order.createPurchase.label", defaultLabel: "Create Purchase Order", href: "/${appName}/purchaseOrder/index", requiredActivitiesAny: [ActivityCode.PLACE_ORDER]],

Create purchase order should also probably be fixed
from purchaseOrder/index to purchaseOrder/create

drodzewicz · 2023-01-11T14:18:03Z

src/js/utils/menu-utils.jsx

+          const {
+            direction,
+            ...otherParams
+          } = mapQueryParamsToObject(search.substring(1, search.length));


use query-string parse instead

import queryString from 'query-string'; const parsed = queryString.parse(search);

kchelstowski · 2023-01-11T21:23:34Z

grails-app/views/order/addComment.gsp

@@ -78,5 +78,6 @@
 			</div>
 		</div>
 	</div>
+	<g:render template="/dashboard/activeSection" model="[section: orderInstance.orderType.name == 'Putaway Order' ? 'inbound' : orderInstance.orderType.name == 'Outbound' ? 'outbound' : 'purchasing']"/>


Rather use:

orderInstance?.orderType?.code == Constants.PUTAWAY_ORDER

or even:

orderInstance?.orderType?.isPutawayOrder()

By the way I'm also concered about comparing order to Outbound - we don't have orderType 'Outbound'.

kchelstowski · 2023-01-11T21:23:49Z

grails-app/views/order/addDocument.gsp

@@ -111,5 +111,6 @@
 			</div>
 		</div>
 	</div>
+	<g:render template="/dashboard/activeSection" model="[section: orderInstance.orderType.name == 'Putaway Order' ? 'inbound' : orderInstance.orderType.name == 'Outbound' ? 'outbound' : 'purchasing']"/>


same as above

kchelstowski · 2023-01-11T21:23:57Z

grails-app/views/order/show.gsp

@@ -303,5 +303,6 @@
              });
            }
        </script>
+    <g:render template="/dashboard/activeSection" model="[section: orderInstance.orderType.name == 'Putaway Order' ? 'inbound' : orderInstance.orderType.name == 'Outbound' ? 'outbound' : 'purchasing']"/>


same as above

kchelstowski · 2023-01-12T09:52:33Z

src/js/components/Layout/menu/Menu.jsx

@@ -32,7 +35,7 @@ const Menu = ({ menuConfig, location }) => {
                <MenuSection
                  section={section}
                  key={`${section.label}-menu-section`}
-                  active={activeSection === section.label}
+                  active={activeSection === section.id}


jmiranda · 2023-01-13T04:51:48Z

I need a bit more time on this one.

jmiranda

I don't think we want to be passing in this section information on every GSP when we already have it available in the Config (i.e. search href by the current URL to get the active section). Let's discuss this one on the tech huddle.

jmiranda · 2023-01-18T16:27:16Z

Tech Huddle discussion

Make as many work implicitly without having to inject the active section
- inventoryItem -> inventory
- product -> product
- order -> purchasing
Stock movement can be hard-coded / injected for now
All other exceptions can wait to another release
activeSection.gsp should be included in megamenu.gsp or custom.gsp
Consider using sitemesh to handle the active section value

When you need to override, use the following convention
stockMovement/show.gsp (add to head)

<g:set var="activeSection">$stockMovement?.stockMovementType?.toLowecase()</g:set>
<content tag="activeSection">${activeSection?:controllerName}</content>

kchelstowski

Don't do any changes until @awalkowiak approves my requests

kchelstowski · 2023-01-20T10:27:38Z

grails-app/views/dashboard/megamenu.gsp

-      if (matchingMenuSection) matchingMenuSection.classList.add('active-section');
-      if (matchingMenuSectionCollapsable) matchingMenuSectionCollapsable.classList.add('active-section');
-    }
+    const urlPartsWithSectionConfig = "${(grailsApplication.config.openboxes.menuSectionsUrlParts as JSON)}";


I feel like it could be better to return it from the backend, from the def megamenu just adding another property to return after menu, configurationSection, megamenuConfig:

menuSectionsUrlParts: grailsApplication.config.openboxes.menuSectionsUrlParts as JSON

and to create a hidden field with id/class, that would allow you to access it from the js script side

@kchelstowski Yup, good suggestion

awalkowiak

One question about the util on the react side plus take a look at Kacpers suggestion

awalkowiak · 2023-01-20T11:14:30Z

src/js/utils/menu-utils.jsx

+import _ from 'lodash';
+import queryString from 'query-string';
+
+export const checkActiveSection = ({


@alannadolny could this util also be simplified to use the new sections config?

I don't think so, because this util has to compare sectionSearch, not only sectionPath. There are some tricky places where the sectionSearch can change the section which should be underlined.

drodzewicz · 2023-01-20T11:15:15Z

grails-app/views/dashboard/megamenu.gsp

+      // check if current section exists in object with wrongly underlined urls
+      const sectionFromJson = Object.keys(parsedUrlPartsWithSection).find(sectionName => {
+        return !!parsedUrlPartsWithSection[sectionName].some(section => path.includes(section));
+      })


Alternatively if we want to avoid this parsing of LinkedHasMap to JSON we can do everything using groovy.
By everything I mean finding the matched section based on config.openboxes.menuSectionUrlParts

So instead of doing it with javascript we can do it with groovy

<g:set var="matchedSection" value="${grailsApplication.config.openboxes.menuSectionsUrlParts.find{ key, val -> val.any{ request?.forwardURI?.contains(it)} }}" /> return menuConfigValues.find(it => it.name === "${matchedSection?.key}");

@alannadolny @kchelstowski @awalkowiak what do you think ?

The solution that Kacper propsed, fixed the problem with replacing &quotes to ".
So now it looks like:
const urlPartsWithSection = $("#menuSectionUrlParts").val();
const parsedUrlPartsWithSection = JSON.parse(urlPartsWithSection);
I think it is much cleaner than doing it in groovy

@drodzewicz I love that idea, but it depends on if we want Alan to spend some more time on this one

alannadolny added 8 commits January 5, 2023 18:09

OBPIH-5273 fix menu in react part

1009adf

OBPIH-5273 fix checkActiveSection function

545a40c

OBPIH-5273 change mapQueryParamsToObject

2a43a11

OBPIH-5273 move menu functions to menu-utils

a7db29d

OBPIH-5273 add tests for menu utils

d6307fb

OBPIH-5273 fix highlighting menu on location configuration pages

624d3e5

OBPIH-5273 fix tests

62a85cc

OBPIH-5273 add function to check path common part

1265887

alannadolny added the work in progress label Jan 10, 2023

alannadolny marked this pull request as draft January 10, 2023 20:21

alannadolny added 2 commits January 11, 2023 00:03

OBPIH-5273 fix underlining in inbounds/outbounds

4a90c55

OBPIH-5273 fix underlining

789e454

alannadolny marked this pull request as ready for review January 11, 2023 00:50

alannadolny requested review from kchelstowski and drodzewicz January 11, 2023 00:50

alannadolny added work in progress and removed work in progress labels Jan 11, 2023

drodzewicz reviewed Jan 11, 2023

View reviewed changes

OBPIH-5273 fix underlining in gsp pages and some react pages

8f222bd

alannadolny requested a review from drodzewicz January 11, 2023 20:08

kchelstowski removed the work in progress label Jan 11, 2023

kchelstowski requested changes Jan 11, 2023

View reviewed changes

kchelstowski requested a review from jmiranda January 11, 2023 21:30

alannadolny added 3 commits January 12, 2023 10:02

OBPIH-5273 fix after review

15c88cc

OBPIH-5273 remove unused translate function

b41e3d8

OBPIH-5273 fix tests

ab53a0f

alannadolny changed the base branch from develop to release/0.8.21 January 12, 2023 09:49

alannadolny requested a review from kchelstowski January 12, 2023 09:49

kchelstowski approved these changes Jan 12, 2023

View reviewed changes

drodzewicz approved these changes Jan 12, 2023

View reviewed changes

jmiranda reviewed Jan 16, 2023

View reviewed changes

jmiranda added the needs discussion label Jan 16, 2023

jmiranda requested a review from awalkowiak January 18, 2023 16:05

jmiranda removed the needs discussion label Jan 18, 2023

OBPIH-5273 remove templates from views, add function to megamenu

67c57a5

alannadolny requested a review from kchelstowski January 20, 2023 10:08

kchelstowski requested changes Jan 20, 2023

View reviewed changes

awalkowiak reviewed Jan 20, 2023

View reviewed changes

drodzewicz reviewed Jan 20, 2023

View reviewed changes

alannadolny added 2 commits January 20, 2023 12:53

OBPIH-5273 fix

feb0901

OBPIH-5273 add one section

070ec68

alannadolny requested review from kchelstowski and awalkowiak January 20, 2023 12:17

awalkowiak approved these changes Jan 20, 2023

View reviewed changes

kchelstowski approved these changes Jan 20, 2023

View reviewed changes

awalkowiak merged commit 08523e8 into release/0.8.21 Jan 20, 2023

awalkowiak deleted the OBPIH-5273 branch January 20, 2023 15:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

OBPIH-5273 Dashboard is underlined in menu on some pages #3770

OBPIH-5273 Dashboard is underlined in menu on some pages #3770

alannadolny commented Jan 10, 2023

drodzewicz Jan 11, 2023

drodzewicz Jan 11, 2023

drodzewicz Jan 11, 2023

kchelstowski Jan 11, 2023

kchelstowski Jan 11, 2023

kchelstowski Jan 11, 2023

kchelstowski Jan 12, 2023

jmiranda commented Jan 13, 2023

jmiranda left a comment

jmiranda commented Jan 18, 2023 •

edited

kchelstowski left a comment

kchelstowski Jan 20, 2023

awalkowiak Jan 20, 2023

awalkowiak left a comment

awalkowiak Jan 20, 2023

alannadolny Jan 20, 2023

drodzewicz Jan 20, 2023

alannadolny Jan 20, 2023 •

edited

kchelstowski Jan 20, 2023

OBPIH-5273 Dashboard is underlined in menu on some pages #3770

OBPIH-5273 Dashboard is underlined in menu on some pages #3770

Conversation

alannadolny commented Jan 10, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmiranda commented Jan 13, 2023

jmiranda left a comment

Choose a reason for hiding this comment

jmiranda commented Jan 18, 2023 • edited

kchelstowski left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

awalkowiak left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alannadolny Jan 20, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jmiranda commented Jan 18, 2023 •

edited

alannadolny Jan 20, 2023 •

edited