Fix workflow breadcrumbs and avoid self links #1132

Closed
opened 2026-04-05 16:25:29 +02:00 by MrUnknownDE · 0 comments
Owner

Originally created by @xuebingli on 3/15/2024

Diagnosis

The cause for #1252 is incorrect paths. Instead of the correct path dashboard/some-uuid/workflows/workflow/another-uuid, an incorrect path of dashboard/some-uuid/workflows/workflow is being used.

Upon investigation, the path generation algorithm is naive - breaking up pathname by / and assigning a portion of pathname based on the array index. b8fcc4c40c/CommonUI/src/Utils/Navigation.ts (L41-L44)

In addition, readers with a keen eye would've noticed that Workflow routes do not follow the convention set by other routes such as Incidents. Workflow routes have an additional /workflow in its path. Thus Workflow routes break the assumption made by the path generation algorithm.

Lastly, self links are widely considered bad UX. #1253 has more details on this.

The Fix

Three approaches were considered.

  1. Make path generation algorithm more complex, handing the convention-breaking Workflow routes.
  2. Make Workflow routes consistent with other routes.
  3. Remove the self link from breadcrumbs, which is usually (but not always) the last link.

This PR implements approach 2 (573e3a0e6af75eb437e97d4b648b9523c9da2b53) and 3 (0342233117a272d7ce7941dbdcc37e6e32cbcf7f).

Approach 2 is chosen for improved code consistency and approach 3 is chosen for better UX. I believe the marginal benefits of approach 1 do not outweigh its increased complexity.

Pull Request Checklist:

  • Please make sure all jobs pass before requesting a review.
  • Put closes #XXXX in your comment to auto-close the issue that your PR fixes (if such).
  • Have you lint your code locally before submission?
  • Did you write tests where appropriate?

Closes #1252
Closes #1253

Screenshots (if appropriate):

The screen recording below shows that

  1. The self link in breadcrumbs is disabled, with the proper cursor style.
  2. The rest of the breadcrumbs continues to work as expected.

https://github.com/OneUptime/oneuptime/assets/1525352/01580cd3-6586-4078-9331-dc31e6bdd6f3

*Originally created by @xuebingli on 3/15/2024* ## Fix Workflow Breadcrumbs and Avoid Self Links ### Diagnosis The cause for #1252 is incorrect paths. Instead of the correct path `dashboard/some-uuid/workflows/workflow/another-uuid`, an incorrect path of `dashboard/some-uuid/workflows/workflow` is being used. Upon investigation, the path generation algorithm is naive - breaking up `pathname` by `/` and assigning a portion of `pathname` based on the array index. https://github.com/OneUptime/oneuptime/blob/b8fcc4c40c071b68545fa902e06297a63e23c647/CommonUI/src/Utils/Navigation.ts#L41-L44 In addition, readers with a keen eye would've noticed that Workflow routes do not follow the convention set by other routes such as Incidents. Workflow routes have an additional `/workflow` in its path. Thus Workflow routes break the assumption made by the path generation algorithm. Lastly, self links are widely considered bad UX. #1253 has more details on this. ### The Fix Three approaches were considered. 1. Make path generation algorithm more complex, handing the convention-breaking Workflow routes. 2. Make Workflow routes consistent with other routes. 3. Remove the self link from breadcrumbs, which is usually (but not always) the last link. This PR implements approach 2 (573e3a0e6af75eb437e97d4b648b9523c9da2b53) and 3 (0342233117a272d7ce7941dbdcc37e6e32cbcf7f). Approach 2 is chosen for improved code consistency and approach 3 is chosen for better UX. I believe the marginal benefits of approach 1 do not outweigh its increased complexity. ### Pull Request Checklist: - [ ] Please make sure all jobs pass before requesting a review. - [x] Put `closes #XXXX` in your comment to auto-close the issue that your PR fixes (if such). - [x] Have you lint your code locally before submission? - [x] Did you write tests where appropriate? ### Related Issues Closes #1252 Closes #1253 ### Screenshots (if appropriate): The screen recording below shows that 1. The self link in breadcrumbs is disabled, with the proper cursor style. 5. The rest of the breadcrumbs continues to work as expected. https://github.com/OneUptime/oneuptime/assets/1525352/01580cd3-6586-4078-9331-dc31e6bdd6f3
Sign in to join this conversation.
No Label
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: github/oneuptime#1132