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.

Sign up for GitHub

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
Merged

Saved trip days styles #1238

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cf0aac9
style(TripMonitoredDays): Shuffle imports and types
binh-dam-ibigroup Jun 13, 2024
022509b
refactor(TripBasicsPane): Apply round styles to availble days
binh-dam-ibigroup Jun 13, 2024
e55cf0b
refactor(TripBasicsPane): Improve styling
binh-dam-ibigroup Jun 14, 2024
8c3cef1
refactor(TripBasicsPane): Tweak styles
binh-dam-ibigroup Jun 14, 2024
d2a1c73
Merge branch 'dev' into saved-trip-days-styles
amy-corson-ibigroup Jul 1, 2024
8d8715f
refactor(TripBasicsPane): Adjust pill layout.
binh-dam-ibigroup Jul 15, 2024
a58aa2d
Merge branch 'dev' into saved-trip-days-styles
binh-dam-ibigroup Jul 15, 2024
7395d62
refactor(TripBasicsPane): Adjust checkbox margin
binh-dam-ibigroup Jul 18, 2024
20bedc9
refactor(TripBasicsPane): Accommodate narrow displays
binh-dam-ibigroup Jul 18, 2024
8b8bfb9
Merge branch 'dev' into saved-trip-days-styles
binh-dam-ibigroup Jul 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 46 additions & 38 deletions lib/components/user/monitored-trip/trip-basics-pane.tsx
View file
Open in desktop
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 •
edited
Loading

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,53 +71,59 @@ 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: auto;
}
& > 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 3px;
}

/* 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: 40%;
transform: rotate(-30deg);
transform-origin: center;
}

label {
flex-grow: 1;
font-weight: inherit;
height: 100%;
line-height: 3rem;
margin: 0;
position: relative;
width: 100%;
}
`
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
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

Thank you for the cleanup!

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