feat: Updated editor teams and team selection pages #3303
Conversation
|
Removed vultr server and associated DNS entries |
|
@ianjon3s Don't sweat too much about failing E2E tests here, I'll jump on and take a look once the PR is ready. It's very likely something very simple to do with the content or location of links changing in the DOM. I'll take a look through and maybe you, me and @RODO94 can debug and work through the fixes together 👍 |
|
@DafyddLlyr I think it may be as simple as me changing a |
ianjon3s
force-pushed
the
ian/team-services-pages-update
branch
from
fe4fb5b to
1f0d1a5
Compare
There was a problem hiding this comment.
A few small comments on, visually looking good though.
This isn't introduced here, but the missing focus styles on the DashboardListItems are a bit more noticeable now. This can be addressed now or later (this is fairly inconsistent across the Editor tbh).
Screen.Recording.2024-06-20.at.16.29.30.mov
| teamThemes: teams_summary { | ||
| slug | ||
| primaryColour: primary_colour | ||
| logo | ||
| } |
There was a problem hiding this comment.
Rather than adding an additional query here, a better pattern (and actually I think the intended one) would be to use the existing relationships to find the corresponding theme details for the relevant team directly in this query (as opposed to having to match them up later using the .find() iterator).
Try this out in Hasura - it should be both faster/more efficient and allow us to simplify our code -
query GetTeams {
teams(order_by: {name: asc}) {
id
name
slug
theme {
primaryColour: primary_colour
logo
}
}
}There was a problem hiding this comment.
I've updated the query to the above which has simplified the component 👍
editor.planx.uk/src/pages/Teams.tsx
Outdated
| const editableTeams = teams.filter((team) => canUserEditTeam(team.slug)); | ||
| const viewOnlyTeams = teams.filter((team) => !canUserEditTeam(team.slug)); |
There was a problem hiding this comment.
nit: You could also do this in a single iteration with a .reduce() or .forEach()
reduce()
const { editableTeams, viewOnlyTeams } = teams.reduce(
(acc, team) => {
canUserEditTeam(team.slug)
? acc.editableTeams.push(team)
: acc.viewOnlyTeams.push(team)
return acc;
},
{ editableTeams: [], viewOnlyTeams: [] }
);forEach()
const editableTeams = [];
const viewOnlyTeams = [];
teams.forEach((team) => (
canUserEditTeam(team.slug)
? editableTeams.push(team)
: viewOnlyTeams.push(team)
));There was a problem hiding this comment.
Thanks for this, I've gone with forEach as it reads better as a piece of logic.
editor.planx.uk/src/pages/Teams.tsx
Outdated
| const renderTeams = (teamsToRender: Array<Team>) => | ||
| teamsToRender.map(({ name, slug }) => { | ||
| const theme = teamTheme.find((t) => t.slug === slug); | ||
| const primaryColour = theme ? theme.primaryColour : "#000"; |
There was a problem hiding this comment.
team_theme.primary_colour is a required field (with a default value) in the database so we should be fine to not have a default here.
It's possible that this is required to get around some TypeScript issues, but this should be resolved by improving the query and dropping the find() (see previous comment).
There was a problem hiding this comment.
I'm now calling this as team.theme.primaryColour without a default value
| flexDirection: "row", | ||
| width: "100%", | ||
| minHeight: `calc(100vh - ${HEADER_HEIGHT}px)`, | ||
| "& > .MuiContainer-root": { |
There was a problem hiding this comment.
I think we should be able to import this classname from MUI 👍
There was a problem hiding this comment.
Good spot, applied
editor.planx.uk/src/pages/Team.tsx
Outdated
| {flow.name} | ||
| </DashboardLink> | ||
| </Typography> | ||
| <HiddenText aria-hidden="true">{flow.slug}</HiddenText> |
There was a problem hiding this comment.
I'm not too sure how much this is really needed? I know it's helpful as a pseudo-search until we have this feature but we can still CTRL+F on flow names?
There was a problem hiding this comment.
Agreed, I've pulled it out. I added as I was getting frustrated when using CTRL+F and typing in a slug, but it's just a case of waiting for new features to be in place.
|
(Also good work on solving the E2E test issues 🙌) |
There was a problem hiding this comment.
Looking good 👍
Before merging this, let's get a prod deploy done first so we can batch a few Editor changes together. There's a PR here to approve which will let us clear the decks a little first - #3310
|
Thanks for the detailed review and suggestions @DafyddLlyr. I've also added focus outline style to the team cards on initial screen. |
|
Thanks @ianjon3s, prod deploy underway so please feel free to merge this 👍 |
What does this PR do?
Introduces style and minor structural changes across Teams and Team pages.
Summary of changes:
In addition to this a
<Dashboard>component has been created to standardise layout and spacing across editor pages (applied to only these two pages for now). This also contains layout styles that make it ready to drop in the editor in-page navigation elements.