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

tweak: Workflow entry change #327

Merged
merged 7 commits into from
May 2, 2024

Conversation

cmaddox5
Copy link
Contributor

Asana task: Perm. Config. Workflow Entry

This PR removes the Configure tab from the sidebar and adds a new button to the Pending page for navigation to the beginning of the Perm Config workflow. No functionality has been changed. The beginning of the workflow can still be accessed via URL.

@cmaddox5 cmaddox5 requested review from a team, jzimbel-mbta and digitalcora and removed request for a team and jzimbel-mbta April 30, 2024 14:23
Copy link
Contributor

@digitalcora digitalcora left a comment

Choose a reason for hiding this comment

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

Just a couple non-blocking questions!

.add-new-button {
height: 48px;
background-color: #8ecdff;
color: #212529;
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we have design tokens for things like this? (i.e. variables)

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 did not for this color and we should have. I double checked designs and this color is showing as primary in Figma. This is not what $button-primary was set to though so I renamed the current $button-primary to $button-secondary (which is how we were using it anyway from what I can tell) and added this color as $button-primary.

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little confused; from the commit, it looks like you meant this color (212529) should be "secondary", and the one above (8ecdff) should be "primary". It also looks like this instance of 212529 is still here, rather than being updated to use the "secondary" variable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Woops, sorry about that. Forgot to do one more Find All before committing the change.

<Container fluid>
<Row className="align-items-center text-white">
<Col>Pending</Col>
<Col className="d-flex justify-content-end">
Copy link
Contributor

Choose a reason for hiding this comment

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

Assuming the class name d-flex applies display: flex, does this do anything in this context? The element only has one child (the button), so there's not really anything for flexbox to lay out.

Copy link
Collaborator

Choose a reason for hiding this comment

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

It looks like it's using justify-content to push the button to the right, so it needs to be a flex container for that to work.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, right, I was thinking the column layout was already handling that. Do we have a justify-self-end utility? Since this column is already a child of a flex layout (or grid layout? unclear to me what fluid actually does).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

fluid is something that React bootstrap has that allows you to force a Container to use all available horizontal space. Docs are here. Without it, it pushes its children towards the center.

Unfortunately, Bootstrap does not have a justify-self-* utility, but I think I found a better way to accomplish the styling here. You're right that the Row is already flex by default. I can add justify-content-between to it and change the responsive breakpoints of the Cols to auto so they only take up as much space as they need. Then the space will be added between the elements and no Col elements need to be flex unnecessarily.

@cmaddox5 cmaddox5 merged commit 8012645 into permanent-configuration May 2, 2024
2 checks passed
@cmaddox5 cmaddox5 deleted the cm/workflow-entry-change branch May 2, 2024 18:10
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.

3 participants