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

Prevent mobile view change on location clear #1085

Merged
merged 14 commits into from
Dec 15, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
17 changes: 14 additions & 3 deletions lib/actions/form.js
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,8 @@ export function checkShouldReplanTrip(autoPlan, isMobile, oldQuery, newQuery) {
(fromChanged && !toChanged) || (!fromChanged && toChanged)
// Check whether a trip should be auto-replanned
const strategy = (isMobile && autoPlan?.mobile) || autoPlan?.default
const locationChangedToBlank =
(oldQuery.from && !newQuery.from) || (oldQuery.to && !newQuery.to)
const shouldReplanTrip =
evaluateAutoPlanStrategy(
strategy,
Expand All @@ -170,6 +172,7 @@ export function checkShouldReplanTrip(autoPlan, isMobile, oldQuery, newQuery) {
(oldQuery.from || oldQuery.to)
return {
fromChanged,
locationChangedToBlank,
oneLocationChanged,
shouldReplanTrip,
toChanged
Expand Down Expand Up @@ -203,8 +206,13 @@ export function formChanged(oldQuery, newQuery) {
const { config, ui } = state.otp
const { autoPlan, debouncePlanTimeMs } = config
const isMobile = coreUtils.ui.isMobile()
const { fromChanged, oneLocationChanged, shouldReplanTrip, toChanged } =
checkShouldReplanTrip(autoPlan, isMobile, oldQuery, newQuery)
const {
fromChanged,
locationChangedToBlank,
oneLocationChanged,
shouldReplanTrip,
toChanged
} = checkShouldReplanTrip(autoPlan, isMobile, oldQuery, newQuery)

dispatch(updateQueryTimeIfLeavingNow())

Expand All @@ -222,7 +230,10 @@ export function formChanged(oldQuery, newQuery) {
// Return to search screen on mobile only if not currently on welcome
// screen (otherwise when the current position is auto-set the screen
// will change unexpectedly).
if (ui.mobileScreen !== MobileScreens.WELCOME_SCREEN) {
if (
ui.mobileScreen !== MobileScreens.WELCOME_SCREEN &&
!locationChangedToBlank
) {
dispatch(setMobileScreen(MobileScreens.SEARCH_FORM))
}
}
Expand Down
2 changes: 2 additions & 0 deletions lib/components/mobile/location-search.js
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ class MobileLocationSearch extends Component {
this.props
const suppressNearby =
otherLocation && otherLocation.category === 'CURRENT_LOCATION'

return (
<MobileContainer>
<MobileNavigationBar
Expand All @@ -59,6 +60,7 @@ class MobileLocationSearch extends Component {
id: 'components.LocationSearch.setOrigin'
})
}
showBackButton
/>
<main tabIndex={-1}>
<div className="location-search mobile-padding">
Expand Down
7 changes: 4 additions & 3 deletions lib/components/mobile/mobile.css
Original file line number Diff line number Diff line change
Expand Up @@ -52,11 +52,12 @@
}

.otp .navbar .mobile-back {
position: fixed;
top: 12px;
left: 15px;
background: transparent;
border: none;
color: white;
amy-corson-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
font-size: 18px;
left: 7px;
position: fixed;
}

.otp .navbar .mobile-close {
Expand Down
18 changes: 15 additions & 3 deletions lib/components/mobile/navigation-bar.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import styled from 'styled-components'
import * as uiActions from '../../actions/ui'
import { accountLinks, getAuth0Config } from '../../util/auth'
import { ComponentContext } from '../../util/contexts'
import { injectIntl } from 'react-intl'
import { StyledIconWrapper } from '../util/styledIcon'
import AppMenu from '../app/app-menu'
import LocaleSelector from '../app/locale-selector'
Expand All @@ -29,6 +30,7 @@ class MobileNavigationBar extends Component {
extraMenuItems: PropTypes.array,
headerAction: PropTypes.element,
headerText: PropTypes.string,
intl: PropTypes.object,
locale: PropTypes.string,
onBackClicked: PropTypes.func,
setMobileScreen: PropTypes.func,
Expand All @@ -51,21 +53,28 @@ class MobileNavigationBar extends Component {
extraMenuItems,
headerAction,
headerText,
intl,
locale,
showBackButton
} = this.props

const backButtonText = intl.formatMessage({ id: 'common.forms.back' })

return (
<header>
<Navbar className="mobile-navbar-container" fixedTop fluid>
<Navbar.Header>
<Navbar.Brand>
{showBackButton ? (
<div className="mobile-back">
<button
amy-corson-ibigroup marked this conversation as resolved.
Show resolved Hide resolved
aria-label={backButtonText}
className="mobile-back"
title={backButtonText}
>
<StyledIconWrapper onClick={this._backButtonPressed}>
<ArrowLeft />
</StyledIconWrapper>
</div>
</button>
) : (
<AppMenu />
)}
Expand Down Expand Up @@ -122,4 +131,7 @@ const mapDispatchToProps = {
setMobileScreen: uiActions.setMobileScreen
}

export default connect(mapStateToProps, mapDispatchToProps)(MobileNavigationBar)
export default connect(
mapStateToProps,
mapDispatchToProps
)(injectIntl(MobileNavigationBar))
6 changes: 6 additions & 0 deletions percy/percy.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,11 +148,17 @@ async function executeTest(page, isMobile, isCallTaker) {
await page.waitForTimeout(300)
// Click the clear button next to it
await page.click('.from-form-control + button')
if (isMobile) {
await page.click('.mobile-back')
}

await page.click('.to-form-control')
await page.waitForTimeout(300)
// Click the clear button next to it
await page.click('.to-form-control + button')
if (isMobile) {
await page.click('.mobile-back')
}

// Fill in new origin
await page.hover('.from-form-control')
Expand Down
Loading