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

feat: add tasks table component and task list page[DET-3221] #652

Merged
merged 19 commits into from
Jun 16, 2020

Conversation

hamidzr
Copy link
Member

@hamidzr hamidzr commented Jun 5, 2020

Description

Screen Capture_select-area_20200604175150

depends on #640 #636 #682

Test Plan

I suspect https://determinedai.atlassian.net/browse/DET-2982 is blocking the storybook added here from rendering.

Commentary (optional)

To keep the PR smaller planning to leave the following for future work:

  • username display for a future PR to keep the PR smaller. the command vs experiment endpoints each return different user/owner data.
  • custom styling to make it look more like Figma design.
  • Duration: commands don't have an end time we might want to either scrap the column or just include for experiments?

@hamidzr hamidzr requested a review from hkang1 June 5, 2020 00:55
@cla-bot cla-bot bot added the cla-signed label Jun 5, 2020
Copy link
Contributor

@hkang1 hkang1 left a comment

Choose a reason for hiding this comment

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

@hamidzr This was probably not clearly outlined, but the tasks list page does not contain experiments and only commands (notebooks, tensorboards, shells and commands).

@hkang1 hkang1 assigned hamidzr and unassigned hkang1 Jun 8, 2020
@hamidzr hamidzr force-pushed the 3221-table-comp-tasklist branch 2 times, most recently from b5fc571 to 7a574d0 Compare June 10, 2020 00:23
@hamidzr hamidzr assigned hkang1 and unassigned hamidzr Jun 10, 2020
@hamidzr
Copy link
Member Author

hamidzr commented Jun 10, 2020

took out the experiments portion and rebased with master

@hkang1
Copy link
Contributor

hkang1 commented Jun 15, 2020

nit: Let's call it TaskTable without the plural s in the name. The table already implies a collection of tasks.

Copy link
Contributor

@hkang1 hkang1 left a comment

Choose a reason for hiding this comment

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

The table code looks really good. Surprisingly very short!
Storybook isn't working yet, probably still dependent on the storybook fix PR.

@hkang1 hkang1 assigned hamidzr and unassigned hkang1 Jun 15, 2020
@hkang1
Copy link
Contributor

hkang1 commented Jun 15, 2020

suggestions: Visual review

  • Add tooltip of the actual time for start time column.
  • Add tooltip of the user's full name on the avatar (probably best to apply directly to Avatar component)
  • Add tooltip of the type on the type icon.
  • Add overflow menu for every row and have a default option of Open so at the basic minimum it won't be empty. Would like to train and get the users to get used to the overflow menu and expect it everywhere they see a task.

@hamidzr
Copy link
Member Author

hamidzr commented Jun 16, 2020

suggestions: Visual review

thanks for the good suggestions!

✔️ * Add tooltip of the actual time for start time column.
✔️ * Add tooltip of the user's full name on the avatar (probably best to apply directly to Avatar component)
✔️ * Add tooltip of the type on the type icon.

  • Add overflow menu for every row and have a default option of Open so at the basic minimum it won't be empty. Would like to train and get the users to get used to the overflow menu and expect it everywhere they see a task.

a bit unsure about the last one 🤔 should we do the same for dashboard task cards?

@hamidzr hamidzr assigned hkang1 and unassigned hamidzr Jun 16, 2020
@hkang1
Copy link
Contributor

hkang1 commented Jun 16, 2020

suggestions: Visual review

thanks for the good suggestions!

✔️ * Add tooltip of the actual time for start time column.
✔️ * Add tooltip of the user's full name on the avatar (probably best to apply directly to Avatar component)
✔️ * Add tooltip of the type on the type icon.

  • Add overflow menu for every row and have a default option of Open so at the basic minimum it won't be empty. Would like to train and get the users to get used to the overflow menu and expect it everywhere they see a task.

a bit unsure about the last one 🤔 should we do the same for dashboard task cards?

synced with @hamidzr will have a disabled/greyed out look for both dashboard and tasks table overflow when there are not actionable items.

Copy link
Contributor

@hkang1 hkang1 left a comment

Choose a reason for hiding this comment

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

Crossed the finish line!
Couple of minor things:

  • using useCallback instead of useMemo in that one area
  • removing duration for tasks table

@hkang1 hkang1 assigned hamidzr and unassigned hkang1 Jun 16, 2020
@hamidzr
Copy link
Member Author

hamidzr commented Jun 16, 2020

#725 I set up a separate PR for adding the no action state to action drop down

@hamidzr hamidzr changed the title feat: add tasks table component [DET-3221] feat: add tasks table component and task list page[DET-3221] Jun 16, 2020
@hamidzr hamidzr merged commit 85a8501 into determined-ai:master Jun 16, 2020
@hamidzr
Copy link
Member Author

hamidzr commented Jun 16, 2020

eh GitHub UI failed.. the test was passing the it showed as failing after merge 🤷‍♂️

@dannysauer dannysauer added this to the 0.12.10 milestone Feb 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants