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

Saved trip days styles #1238

Merged
merged 10 commits into from
Jul 22, 2024
86 changes: 47 additions & 39 deletions lib/components/user/monitored-trip/trip-basics-pane.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Ban } from '@styled-icons/fa-solid/Ban'
import { connect } from 'react-redux'
import {
ControlLabel,
FormControl,
FormGroup,
Glyphicon,
HelpBlock,
ProgressBar,
Radio
Expand All @@ -26,14 +26,15 @@ import {
} from '../../../util/monitored-trip'
import { AppReduxState } from '../../../util/state-types'
import { FieldSet } from '../styled'
import { getBaseColor, RED_ON_WHITE } from '../../util/colors'
import { getErrorStates } from '../../../util/ui'
import { ItineraryExistence, MonitoredTrip } from '../types'
import FormattedDayOfWeek from '../../util/formatted-day-of-week'
import FormattedDayOfWeekCompact from '../../util/formatted-day-of-week-compact'
import FormattedValidationError from '../../util/formatted-validation-error'
import InvisibleA11yLabel from '../../util/invisible-a11y-label'

import { BORDER_COLOR } from './trip-summary-pane'
import { MonitoredDayCircle } from './trip-monitored-days'
import TripStatus from './trip-status'
import TripSummary from './trip-duration-summary'

Expand All @@ -55,6 +56,9 @@ interface State {

// Styles.
const AvailableDays = styled(FieldSet)`
display: flex;
gap: 4px;

Comment on lines 58 to +61
Copy link
Contributor

@amy-corson-ibigroup amy-corson-ibigroup Jul 15, 2024

Choose a reason for hiding this comment

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

These labels and inputs aren't appearing vertically centered to me, especially when there's a unavailable day
image

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I do see that on Firefox but not Chrome... It is actually the text that is slightly offset vertically, the icon and the checkbox alignment are fine.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

You see a similar offset with the regular input controls too:
image

// Targets the formik checkboxes to provide better contrast on focus styles
input {
&:focus-visible,
Expand All @@ -67,54 +71,60 @@ const AvailableDays = styled(FieldSet)`
}
}
& > span {
border: 1px solid ${BORDER_COLOR};
border-left: none;
align-items: center;
border-radius: 3rem;
box-sizing: border-box;
display: inline-block;
height: 3em;
max-width: 150px;
min-width: 14.28%;
display: inline-flex;
flex-direction: row-reverse;
height: 3rem;
min-width: 4.5rem;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we adjust the width/padding on these to make them more like pills? They're looking a little squished as is.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

How do the changes from 8d8715f look like?

Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good thank you! I think the margin could be adjusted to something like 0 7px 0 2px to look a little more even?

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 in 7395d62

position: relative;
text-align: center;
width: 5rem;
}
& > span:first-of-type {
border-left: 1px solid ${BORDER_COLOR};
}

.glyphicon {
svg {
color: ${RED_ON_WHITE};
display: none;
/* Remove top attribute set by Bootstrap. */
top: inherit;
}

input {
display: block;
width: 1.3rem;
}

input,
.glyphicon {
bottom: 6px;
position: absolute;
width: 100%;
svg {
flex-shrink: 0;
/* Remove bootstrap's vertical margin */
margin: 0 7px 0 2px;
}

/* Check boxes for disabled days are replaced with the cross mark. */
input[disabled] {
clip: rect(0, 0, 0, 0);
height: 0;
margin: 0;
width: 0;
z-index: -1;
display: none;
}
input[disabled] ~ .glyphicon {
input[disabled] ~ svg {
display: block;
}

/* Make labels occupy the whole space, so the entire block is clickable. */
/* Add oblique strike for disabled days */
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this addition!

.disabled-day::after {
border-top: 2px solid ${RED_ON_WHITE};
content: '';
left: 0;
position: absolute;
right: 0;
top: 45%;
transform: rotate(-30deg);
transform-origin: center;
}

label {
flex-grow: 1;
font-weight: inherit;
height: 100%;
width: 100%;
line-height: 3rem;
margin: 0;
position: relative;
text-align: center;
}
`

Expand Down Expand Up @@ -304,11 +314,7 @@ class TripBasicsPane extends Component<TripBasicsProps, State> {
day,
finalItineraryExistence
)
const boxClass = isDayDisabled
? 'alert-danger'
: monitoredTrip[day]
? 'bg-primary'
: ''
const labelClass = isDayDisabled ? 'disabled-day' : ''
const notAvailableText = isDayDisabled
? intl.formatMessage(
{
Expand All @@ -320,10 +326,12 @@ class TripBasicsPane extends Component<TripBasicsProps, State> {
)
: ''

const baseColor = getBaseColor()
return (
<span
className={boxClass}
<MonitoredDayCircle
baseColor={baseColor}
key={day}
monitored={!isDayDisabled && monitoredTrip[day]}
title={notAvailableText}
>
<Field
Expand All @@ -334,20 +342,20 @@ class TripBasicsPane extends Component<TripBasicsProps, State> {
name={day}
type="checkbox"
/>
<Ban aria-hidden />
<label htmlFor={day}>
<InvisibleA11yLabel>
<FormattedDayOfWeek day={day} />
</InvisibleA11yLabel>
<span aria-hidden>
<span aria-hidden className={labelClass}>
{/* The abbreviated text is visual only. Screen readers should read out the full day. */}
<FormattedDayOfWeekCompact day={day} />
</span>
</label>
<Glyphicon aria-hidden glyph="ban-circle" />
<InvisibleA11yLabel>
{notAvailableText}
</InvisibleA11yLabel>
</span>
</MonitoredDayCircle>
)
})}
</AvailableDays>
Expand Down
12 changes: 5 additions & 7 deletions lib/components/user/monitored-trip/trip-monitored-days.tsx
Original file line number Diff line number Diff line change
@@ -1,14 +1,12 @@
import { FormattedList, FormattedMessage, useIntl } from 'react-intl'

import InvisibleA11yLabel from '../../util/invisible-a11y-label'
import { FormattedList, FormattedMessage } from 'react-intl'
import React from 'react'
import styled from 'styled-components'

import { ALL_DAYS } from '../../../util/monitored-trip'
import { getBaseColor } from '../../util/colors'
import FormattedDayOfWeek from '../../util/formatted-day-of-week'
import FormattedDayOfWeekCompact from '../../util/formatted-day-of-week-compact'

import styled from 'styled-components'
import InvisibleA11yLabel from '../../util/invisible-a11y-label'

interface Props {
days: string[]
Expand All @@ -19,7 +17,7 @@ const DayCircleContainer = styled.div`
gap: 4px;
`

const MonitoredDayCircle = styled.span<{
export const MonitoredDayCircle = styled.span<{
baseColor: string
monitored: boolean
}>`
Expand All @@ -41,7 +39,7 @@ const MonitoredDayCircle = styled.span<{
}
`

const MonitoredDays = ({ days }: Props) => {
const MonitoredDays = ({ days }: Props): JSX.Element => {
const monitoredDaysList = (
<FormattedList
type="conjunction"
Expand Down
Loading