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

Adding support for assigning people in tasks and much more #188

Merged
merged 54 commits into from
Nov 2, 2019

Conversation

vogtn
Copy link
Contributor

@vogtn vogtn commented Oct 18, 2019

PR Type

  • Bugfix
  • Feature

Description of the changes

New features:

  • mgt-tasks:
    • users can now select assigned people or update existing tasks with assigned people
    • assigned people now have a person-card on hover

Bug fixes:

  • A fix for when removing people from mgt-people-picker or mgt-people, the image for the people was not updating.

New properties, events, templates:

  • mgt-people:
    • no-people template
    • person-card attribute
    • user-ids attribute
  • mgt-tasks
    • task template
    • task-details template
    • isNewTaskVisible property (thanks @BenjiFischman)
    • taskClick event

General cleanup of code and styles for mgt-tasks

PR checklist

  • Added tests and all passed
  • All public classes and methods have been documented
  • Added appropriate documentation
  • License header has been added to all new source files
  • Contains NO breaking changes

Other information

Copy link
Contributor

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

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

  • image
    The +1 in the people looks weird in tasks

  • Clicking on the people in tasks when people picker is open should close the people picker

  • If people in people-picker don't change, then the task should not be updated - now it updates every time the people picker is closed and sometimes graph errors show up

  • person-card is enabled for people in new task

src/Graph.ts Outdated Show resolved Hide resolved
@vogtn
Copy link
Contributor Author

vogtn commented Nov 1, 2019

  • The +1 in the people looks weird in tasks

fixed, and spacing between calendar: fa32cdc

  • person-card is enabled for people in new task

removed: 933c4bc

  • If people in people-picker don't change, then the task should not be updated - now it updates every time the people picker is closed and sometimes graph errors show up

updated: 240d371

Copy link
Contributor

@nmetulev nmetulev left a comment

Choose a reason for hiding this comment

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

Few more comments

  • the taskClick event is only fired when a click on the people - the oposite should be true. The event should fire when I click on the task anywhere (except areas where there is an action/button - people, completing, options, etc)

  • I get an exception when changing the assignment of a task:
    image

this._currentTargetDresser = this.initialId;
} else if (this.dataSource === 'todo') {
this._currentTargetDrawer = this.initialId;
window.addEventListener('click', (event: MouseEvent) => {
Copy link
Contributor

Choose a reason for hiding this comment

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

Per lit-element docs, events on window should be set up in connectedCallback and removed in disconnectedCallback.

If your component adds an event listener to anything except itself or its children–for example, to Window, Document, or some element in the main DOM–you should add the listener in connectedCallback and remove it in disconnectedCallback.

src/components/mgt-tasks/mgt-tasks.ts Outdated Show resolved Hide resolved
src/components/mgt-tasks/mgt-tasks.ts Outdated Show resolved Hide resolved
if (picker) {
for (const person of picker.selectedPeople) {
if (picker.selectedPeople.length) {
peopleObj[person.id] = { '@odata.type': 'microsoft.graph.plannerAssignment', orderHint: 'string !' };
Copy link
Contributor

Choose a reason for hiding this comment

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

You are using this object in multiple places - makes sense to define it once and reuse it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Refactored peopleObj: 3d33dd2

Copy link
Contributor

Choose a reason for hiding this comment

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

sorry, should have been more clear, I meant the {'odata.type.... object

src/components/mgt-tasks/mgt-tasks.ts Outdated Show resolved Hide resolved
src/components/mgt-tasks/mgt-tasks.ts Outdated Show resolved Hide resolved
src/components/mgt-tasks/mgt-tasks.ts Outdated Show resolved Hide resolved
src/components/mgt-tasks/mgt-tasks.ts Outdated Show resolved Hide resolved
src/components/mgt-tasks/mgt-tasks.ts Outdated Show resolved Hide resolved
src/components/mgt-tasks/task-sources.ts Outdated Show resolved Hide resolved
@nmetulev nmetulev changed the title Nivogt/v2/task peoplepicker Adding support for assigning people in tasks and much more Nov 1, 2019
@vogtn
Copy link
Contributor Author

vogtn commented Nov 2, 2019

  • the taskClick event is only fired when a click on the people - the oposite should be true. The event should fire when I click on the task anywhere (except areas where there is an action/button - people, completing, options, etc)

fixed here: ef2c2d9

@vogtn
Copy link
Contributor Author

vogtn commented Nov 2, 2019

  • Clicking on the people in tasks when people picker is open should close the people picker

fixed here: bc6bf22

@nmetulev nmetulev merged commit cce5746 into master Nov 2, 2019
@nmetulev nmetulev deleted the nivogt/v2/task-peoplepicker branch November 2, 2019 04:32
@nmetulev nmetulev mentioned this pull request Nov 2, 2019
14 tasks
shweaver-MSFT pushed a commit that referenced this pull request Jun 8, 2020
* people-picker in task component basic functionality. TODO: filter people

* [mgt-task] fixing styles of picker

* [mgt-tasks] default state if nobody is assigned

* fixing errors

* [mgt-tasks] adding deletion or removing selected people

* [Graph] adding assign people promise to planner

* always show assigned to me

* renamed drawers and dressers

* filtering of tasks when target-id is set and clean up of mgt-tasks

* added tasks templates and fied styles and utils

* [mgt-tasks] updating method for handling people picker hiding

* updating picker styles

* [mgt-task] fixing null selected people case

* [mgt-tasks] updating styles for better scss scaffolding and new logic for handling render of people-picker

* task source documentation

* [mgt-tasks] assings to default if no people are provided

* [mgt-tasks] picker on new task issue

* adding property to mgt-people for user id selectionfor use in tasks

* minor layout fix for new task

* [mgt-people] adding personCardInteraction property to people

* restoring index.html

* [mgt-people] adding 'no-people' template

* [mgt-tasks] updating task and picker to inherit people from each other

* [PersonCardInteraction] refactoring personcard interactions for specific definitions of hover and click for task components

* removing and updating necessary imports

* [mgt-people] refactor peopleid promise into promise.all

* [mgt-tasks mgt-people] removed default html for no-people and moved template to task. updated picker hiding method

* [mgt-tasks] adding taskClick custom event

* fixed default person card on people

* Added setter and getter for property isNewTaskVisible

* [Graph.ts] changing order of assignPeopleToTask parameters

* [mgt-tasks] fixing mgt-people styles

* [mgt-tasks] disabled person-card inside of newTask people

* [mgt-tasks] adding isEqual if picker chosen people aren't different not to update

* assingPeopletoTask rather than person

* rename nopeople template and click handler

* Fix for images in people and people-picker not updating when people change

* fix for person card in tasks

* [mgt-tasks] refactor show and hid picker, removing self assigned task

* [mgt-tasks] refactor peopleObj

* [mgt-tasks] fixing typing in assign people and sort

* [mgt-tasks] re-work of taskClick event

* [mgt-tasks] allowing click on people to save and hide people-picker

* update taskClick event with raw task results, and minor fixes
shweaver-MSFT pushed a commit that referenced this pull request Jun 10, 2020
* people-picker in task component basic functionality. TODO: filter people

* [mgt-task] fixing styles of picker

* [mgt-tasks] default state if nobody is assigned

* fixing errors

* [mgt-tasks] adding deletion or removing selected people

* [Graph] adding assign people promise to planner

* always show assigned to me

* renamed drawers and dressers

* filtering of tasks when target-id is set and clean up of mgt-tasks

* added tasks templates and fied styles and utils

* [mgt-tasks] updating method for handling people picker hiding

* updating picker styles

* [mgt-task] fixing null selected people case

* [mgt-tasks] updating styles for better scss scaffolding and new logic for handling render of people-picker

* task source documentation

* [mgt-tasks] assings to default if no people are provided

* [mgt-tasks] picker on new task issue

* adding property to mgt-people for user id selectionfor use in tasks

* minor layout fix for new task

* [mgt-people] adding personCardInteraction property to people

* restoring index.html

* [mgt-people] adding 'no-people' template

* [mgt-tasks] updating task and picker to inherit people from each other

* [PersonCardInteraction] refactoring personcard interactions for specific definitions of hover and click for task components

* removing and updating necessary imports

* [mgt-people] refactor peopleid promise into promise.all

* [mgt-tasks mgt-people] removed default html for no-people and moved template to task. updated picker hiding method

* [mgt-tasks] adding taskClick custom event

* fixed default person card on people

* Added setter and getter for property isNewTaskVisible

* [Graph.ts] changing order of assignPeopleToTask parameters

* [mgt-tasks] fixing mgt-people styles

* [mgt-tasks] disabled person-card inside of newTask people

* [mgt-tasks] adding isEqual if picker chosen people aren't different not to update

* assingPeopletoTask rather than person

* rename nopeople template and click handler

* Fix for images in people and people-picker not updating when people change

* fix for person card in tasks

* [mgt-tasks] refactor show and hid picker, removing self assigned task

* [mgt-tasks] refactor peopleObj

* [mgt-tasks] fixing typing in assign people and sort

* [mgt-tasks] re-work of taskClick event

* [mgt-tasks] allowing click on people to save and hide people-picker

* update taskClick event with raw task results, and minor fixes
shweaver-MSFT pushed a commit that referenced this pull request Jun 10, 2020
* people-picker in task component basic functionality. TODO: filter people

* [mgt-task] fixing styles of picker

* [mgt-tasks] default state if nobody is assigned

* fixing errors

* [mgt-tasks] adding deletion or removing selected people

* [Graph] adding assign people promise to planner

* always show assigned to me

* renamed drawers and dressers

* filtering of tasks when target-id is set and clean up of mgt-tasks

* added tasks templates and fied styles and utils

* [mgt-tasks] updating method for handling people picker hiding

* updating picker styles

* [mgt-task] fixing null selected people case

* [mgt-tasks] updating styles for better scss scaffolding and new logic for handling render of people-picker

* task source documentation

* [mgt-tasks] assings to default if no people are provided

* [mgt-tasks] picker on new task issue

* adding property to mgt-people for user id selectionfor use in tasks

* minor layout fix for new task

* [mgt-people] adding personCardInteraction property to people

* restoring index.html

* [mgt-people] adding 'no-people' template

* [mgt-tasks] updating task and picker to inherit people from each other

* [PersonCardInteraction] refactoring personcard interactions for specific definitions of hover and click for task components

* removing and updating necessary imports

* [mgt-people] refactor peopleid promise into promise.all

* [mgt-tasks mgt-people] removed default html for no-people and moved template to task. updated picker hiding method

* [mgt-tasks] adding taskClick custom event

* fixed default person card on people

* Added setter and getter for property isNewTaskVisible

* [Graph.ts] changing order of assignPeopleToTask parameters

* [mgt-tasks] fixing mgt-people styles

* [mgt-tasks] disabled person-card inside of newTask people

* [mgt-tasks] adding isEqual if picker chosen people aren't different not to update

* assingPeopletoTask rather than person

* rename nopeople template and click handler

* Fix for images in people and people-picker not updating when people change

* fix for person card in tasks

* [mgt-tasks] refactor show and hid picker, removing self assigned task

* [mgt-tasks] refactor peopleObj

* [mgt-tasks] fixing typing in assign people and sort

* [mgt-tasks] re-work of taskClick event

* [mgt-tasks] allowing click on people to save and hide people-picker

* update taskClick event with raw task results, and minor fixes
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.

None yet

3 participants