Skip to content

fix: remove access column header #2976

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

Merged
merged 2 commits into from
Jul 14, 2022

Conversation

AbhineetJain
Copy link
Contributor

This PR removes the Access column header on the workspace page and aligns the access buttons to the right of the table row.

Additionally, we also remove the table column border when the resource does not have an agent.

Subtasks

  • Remove Access table heading
  • Right align access buttons
  • Remove border from table column when no agents are present

Screenshot

Screen Shot 2022-07-13 at 12 23 26 PM

Fixes #2974

@AbhineetJain AbhineetJain requested a review from a team as a code owner July 13, 2022 19:23
@AbhineetJain
Copy link
Contributor Author

Interestingly, Storybook isn't picking up the right alignment changes. 🤔

@BrunoQuaresma
Copy link
Collaborator

@AbhineetJain one small thing, can you make the paddingLeft of the "Agent" column the same as the "Resource" column?

@@ -184,5 +183,6 @@ const useStyles = makeStyles((theme) => ({
display: "flex",
gap: theme.spacing(0.5),
flexWrap: "wrap",
justifyContent: "right",
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you should use flex-end. I think "right" is not well supported. This should be the reason for Chromatic getting different styles.

@@ -80,7 +79,7 @@ export const Resources: FC<ResourcesProps> = ({
if (!agent) {
return (
<TableRow key={`${resource.id}-${agentIndex}`}>
<TableCell className={styles.resourceNameCell}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can you check if we are using this anywhere else? If not, I think we should remove the resourceNameCell from the makeStyles.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We're using it to show the border when agents exist. Removed it from the case when no agents exist.

Copy link
Collaborator

@BrunoQuaresma BrunoQuaresma left a comment

Choose a reason for hiding this comment

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

LGTM, just minor stuff before the merge.

@AbhineetJain AbhineetJain merged commit bf1af21 into main Jul 14, 2022
@AbhineetJain AbhineetJain deleted the abhineetjain/2974-remove-access-column branch July 14, 2022 00:59
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.

Remove the Access column and right align the access buttons
2 participants