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

🐛 Add task detail links to tasks page #1980

Merged
merged 3 commits into from
Jun 26, 2024

Conversation

rszwajko
Copy link
Collaborator

@rszwajko rszwajko commented Jun 24, 2024

  1. extract base component to re-use for Analysis Details and Task Details
  2. add 2 new routes:
    a) /tasks/:taskId
    b) /tasks/:taskId/attachments/:attachmentId
  3. user can enter details screen by:
    a) task status name (link)
    b) "Task details" action (per row kebab actions)
  4. persist page state in session storage for Applications
    and Task Manager pages to fix problems with returning
    from task details page.

Resolves: #1975

@rszwajko
Copy link
Collaborator Author

Details screen available via link and action

image

Task details - log view

image

Task details - attachment view

image

Copy link

codecov bot commented Jun 24, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 42.23%. Comparing base (b654645) to head (4eff123).
Report is 178 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1980      +/-   ##
==========================================
+ Coverage   39.20%   42.23%   +3.02%     
==========================================
  Files         146      170      +24     
  Lines        4857     5451     +594     
  Branches     1164     1364     +200     
==========================================
+ Hits         1904     2302     +398     
- Misses       2939     3032      +93     
- Partials       14      117     +103     
Flag Coverage Δ
client 42.23% <100.00%> (+3.02%) ⬆️
server ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@rszwajko rszwajko marked this pull request as ready for review June 24, 2024 18:02
Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

What looks good?

  • code changes look good
  • navigation from the table looks good
  • URLs look good
  • attached files and merged views look good

Breadcrumbs

What about having them include the "Task ${task.id}" text included? So for a given URL like "/tasks/7" they look like:

Tasks > Task 7 > Task details

Title

The title shows the name of the task. That is not enough to positively identify the task. If you create an application "Bear" w/o sources initially, and then edit it to include sources, there will be multiple tasks named "Bear-language-discovery".

Maybe a title more like:

"Task details for task ${task.id}, ${task.name}"

@rszwajko
Copy link
Collaborator Author

rszwajko commented Jun 25, 2024

@sjd78
Below the new wording. Note that Task 9 is last segment and is inactive (not a link) because it's the current page. Active links denote previous pages. At least this is my understanding of PF design guide .

image

On Analysis details screen situation is a bit different - we used "fake" application page (main table with details drawer opened). There "Analysis details" is the current page.
image

1. extract base component to re-use for Analysis Details and Task
   Details
2. add 2 new routes:
  a) /tasks/:taskId
  b) /tasks/:taskId/attachments/:attachmentId
3. user can enter details screen by:
  a) task status name (link)
  b) "Task details" action (per row kebab actions"

Signed-off-by: Radoslaw Szwajkowski <[email protected]>
@rszwajko rszwajko force-pushed the linkToDetails branch 2 times, most recently from 46cb9b9 to 63af805 Compare June 26, 2024 13:01
@rszwajko rszwajko requested a review from sjd78 June 26, 2024 13:17
@rszwajko
Copy link
Collaborator Author

@sjd78
one important fix was added - persisting page state in session storage:paging, sorting and filtering (Application page only). This should fix problems with returning back to main page from details pages.

Copy link
Member

@sjd78 sjd78 left a comment

Choose a reason for hiding this comment

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

LGTM

@rszwajko rszwajko merged commit 91400ed into konveyor:main Jun 26, 2024
13 checks passed
@sjd78 sjd78 added this to the v0.5.0 milestone Jul 11, 2024
rszwajko added a commit that referenced this pull request Jul 31, 2024
Fix regression after: #1980
Resolves: https://issues.redhat.com/browse/MTA-1390

Initial filter values work best if the table is not persisting state
between reloads (default strategy, component state). If the table uses
session or local storage then any existing filter overwrites the default
value. From the end user perspective the functionality works only on the
first load with a clean browser (no previous state).

Switching to URL filter persistence provides the same functionality and
ensures we have only one source of filters.

Reference-Url:  #1698

Signed-off-by: Radoslaw Szwajkowski <[email protected]>
rszwajko added a commit to rszwajko/tackle2-ui that referenced this pull request Aug 1, 2024
Fix regression after: konveyor#1980
Resolves: https://issues.redhat.com/browse/MTA-1390

Initial filter values work best if the table is not persisting state
between reloads (default strategy, component state). If the table uses
session or local storage then any existing filter overwrites the default
value. From the end user perspective the functionality works only on the
first load with a clean browser (no previous state).

Switching to URL filter persistence provides the same functionality and
ensures we have only one source of filters.

Reference-Url:  konveyor#1698

Signed-off-by: Radoslaw Szwajkowski <[email protected]>
sjd78 pushed a commit that referenced this pull request Aug 1, 2024
#2033) (#2038)

Backport-of:  (#2033)
Fix regression after: #1980
Resolves: https://issues.redhat.com/browse/MTA-1390

Initial filter values work best if the table is not persisting state
between reloads (default strategy, component state). If the table uses
session or local storage then any existing filter overwrites the default
value. From the end user perspective the functionality works only on the
first load with a clean browser (no previous state).

Switching to URL filter persistence provides the same functionality and
ensures we have only one source of filters.

Reference-Url:  #1698

Signed-off-by: Radoslaw Szwajkowski <[email protected]>
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.

[BUG] Add a method to view a task's log from the task table
2 participants