Skip to content

Conversation

@mvolkmann
Copy link
Collaborator

@mvolkmann mvolkmann commented May 10, 2024

You'll have to expand the diff for TeamReviews.jsx to see my comments because that one is a large diff.

}

.member-selector-card .member-selector-card-title {
font-size: 1.5em;
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Adjusted some font sizes to improve UI.

Avatar,
Card,
CardHeader,
Collapse,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lots of UI functionality was removed from this component per Michael.
We no longer display a list of the selected members and only display the number of selected members.


const addMembers = membersToAdd => {
onChange([...selected, ...membersToAdd]);
const replaceSelectedMembers = members => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We now replace the complete set of selected members rather than adding to it.

onSelect={handleNewRequest}
onClose={handleCloseNewRequest}
/>
<MemberSelectorDialog
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is used to display the team members associated with this review period.
It also allows more to be added and existing selections to be removed.

onClose={() => setDialogOpen(false)}
onSubmit={membersToAdd => setTeamMembers(membersToAdd)}
/>
<Dialog
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is used for confirmation before a review period can be deleted.

@@ -1,7 +1,8 @@
import dayjs from 'dayjs';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This seems to be required for passing dates to the MUI DatePicker component.

@@ -1,7 +1,8 @@
import dayjs from 'dayjs';
import React, { useRef, useEffect, useState } from 'react';
import { LocalizationProvider } from '@mui/x-date-pickers/LocalizationProvider';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was just moved down a few lines.

import React, { useCallback, useContext, useEffect, useState } from 'react';

import {
Archive,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Lots of UI functionality was moved from this component to TeamReviews.jsx.

}
key={`period-${id}`}
>
<ListItem key={`period-${id}`}>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This simpler ListItem replaces the one above.

export const ADD_TEAM = '@@check-ins/add_team';
export const DELETE_MEMBER_PROFILE = '@@check-ins/delete_member_profile';
export const DELETE_MEMBER_SKILL = '@@check-ins/delete_member_skill';
export const DELETE_REVIEW_PERIOD = '@@check-ins/delete_review_period';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Two new actions were added.

state.guilds.sort((a, b) => a.name.localeCompare(b.name));
state.guilds = [...state.guilds];
break;
case UPDATE_REVIEW_PERIODS:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was just moved down.

case ADD_REVIEW_PERIOD:
state.reviewPeriods = [...state.reviewPeriods, action.payload];
break;
case DELETE_REVIEW_PERIOD:
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Two new actions were implemented.

title="Select PDLs"
selected={selectedPdls}
onChange={setSelectedPdls}
listHeight={180}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This prop is no longer needed.

<div className="team-skill-report-page">
<MemberSelector
className="team-skill-member-selector"
listHeight={300}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This prop is no longer needed.

Tooltip,
Typography
} from '@mui/material';
import AddIcon from '@mui/icons-material/Add';
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Some of these icons are no longer used and access to the ones that are was simplified by using their given names.

) : (
<TeamReviews periodId={selectedPeriod} />
<TeamReviews
onBack={() => onPeriodSelected(null)}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This allows going back to the ReviewPeriods page from the TeamReviews page.

teamMembers: PropTypes.arrayOf(
PropTypes.shape({
id: PropTypes.string,
firstName: PropTypes.string,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These props are not used.

id: PropTypes.string,
firstName: PropTypes.string,
lastName: PropTypes.string
onBack: PropTypes.func
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is a new prop for returning to the ReviewPeriods page.


return (
<Root>
<Tooltip title="Back">
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

New "Back" button for returning to the ReviewPeriods page.

Copy link
Contributor

@S78901 S78901 left a comment

Choose a reason for hiding this comment

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

Looks good!

@mvolkmann mvolkmann merged commit 49b0b1c into develop May 10, 2024
@mkimberlin mkimberlin deleted the feature-2284-review-period-employees branch August 27, 2024 18:44
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.

4 participants