Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Streamline search for solution process #18500

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

cconard96
Copy link
Contributor

@cconard96 cconard96 commented Dec 7, 2024

Checklist before requesting a review

  • I have read the CONTRIBUTING document.
  • I have performed a self-review of my code.
  • I have added tests that prove my fix is effective or that my feature works.

Description

When using object locks, there was an opportunity for a technician to lose the lock on a ticket when searching for a solution because the process redirects out of the ticket for the solution search/selection part.

This PR moves the solution selection to a modal and allows adding the article content directly to the solution content without any redirects/page reloads.

Fixes #18490

src/KnowbaseItem.php Show resolved Hide resolved
src/Glpi/Controller/KBController.php Outdated Show resolved Hide resolved
@cedric-anne cedric-anne requested a review from orthagh December 10, 2024 09:15
@cconard96 cconard96 marked this pull request as draft December 10, 2024 22:51
@orthagh
Copy link
Contributor

orthagh commented Dec 13, 2024

Regarding UX.
I like the idea of getting the kb directly displayed in a modal and it's a good starting point.
But the view of the content of an article is bad currently.
We move to a full layout of GLPI if we click on an article's title, and we don't have anymore a "use as solution" button.
image

Also, this part is very badly displayed:
image

I think we should probably remove it completely in the modal

@cconard96
Copy link
Contributor Author

There are a few things that aren't handled yet like linking the KB article in addition to just copying the content, pagination, etc.

This work is going to need be more complex so I put it to draft.

@cconard96
Copy link
Contributor Author

It is rough, but here is a replacement UI proposal for searching solutions:
Selection_380

I pushed commits that add this and it should be functional. The only main thing missing is pagination.

When you click the button to search for a solution, it presents you with a list of matching articles rather than the table that was there previously. Each result has two buttons; A button to use it as the solution and a button to preview the article. If you preview an article, the results are hidden and the "full" form of the article is shown in its place in the modal. A button at the top of the preview lets you go back to the results. Searching from inside this modal refreshes the results via AJAX.

@orthagh
Copy link
Contributor

orthagh commented Dec 16, 2024

It should be good like this.

@cconard96 cconard96 marked this pull request as ready for review January 4, 2025 18:53
@cconard96 cconard96 force-pushed the enhance/search_solution branch from 0695299 to f882458 Compare January 8, 2025 15:17
@cconard96 cconard96 requested a review from cedric-anne January 8, 2025 15:19
@cedric-anne
Copy link
Member

@orthagh Is this PR OK for you ?

Copy link
Contributor

@orthagh orthagh left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Almost
There is UI polishing to do:

  • KB articles with HTML elements (images, bold, titles, etc) appear raw. At least, I expect to have the new lines respected, if you prefer to display only text.
    image

  • Maybe we can truncate (rather overflow hidden) articles in the list searching for a solution and avoid them being higher than 100-150px

  • mini button "Use as solution" and "preview" should have tooltips.

  • avoid double border

  • The preview mode of an article has some quirks

    • can avoid double border also here
    • We miss a "Use as solution" button

    image

@cconard96
Copy link
Contributor Author

* Maybe we can truncate (rather overflow hidden) articles in the list searching for a solution and avoid them being higher than 100-150px

The truncation is based on $CFG_GLPI['cut'], although maybe it could be based on the constant GLPI_TEXT_MAXSIZE.

@orthagh
Copy link
Contributor

orthagh commented Jan 13, 2025

maybe it could be based...

I'm not sure, we can just set a fixed limit for this feature. I don't see any advantage to let the user change the height here.

@cconard96 cconard96 force-pushed the enhance/search_solution branch from f882458 to 3fc35ad Compare January 14, 2025 00:35
@orthagh
Copy link
Contributor

orthagh commented Jan 14, 2025

UI/UX review:

  • add a margin here or better add the primary action on right (margin left auto)
    image
  • "Use as solution" should be btn-primary class
  • Remove justify-content-center on search header and remove size=50 from search input
    image
  • Move the "Showing 1 to 4 of 4 rows" message at bottom
  • I think we can reduce more the max height of an article in the list (150px seems good)
    But I'm not fan of the current way of designing an ellipsis. I suggest using line-clamp property set to 6 or 7 lines, it should match the 150px (and replace it)
    .solution-content-preview {
      ...
      display: -webkit-box;
      -webkit-line-clamp: 7;
      -webkit-box-orient: vertical;
    }
    image
    (note browser support seems ok: https://caniuse.com/css-line-clamp)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

When searching for a solution, the ticket loses the Lock status
5 participants