Conversation
… checkbox selecting or unselect
| bikeOnly ? 'bike' : walkOnly ? 'walk' : '' | ||
| }`} | ||
| id={appElement} | ||
| style={{ height: isTimepickerSelected ? '380px' : '250px' }} |
There was a problem hiding this comment.
This could be moved sass file by providing a class name here if the timepicker is selected.
| > | ||
| <div className="background-container">{drawBackgroundIcon()}</div> | ||
| <div className="control-panel-container"> | ||
| <div className="control-panel-container" style={{ position: 'relative' }}> |
There was a problem hiding this comment.
This style could be potentially moved to sass as well.
|
|
||
| <div | ||
| className="embedded-search-button-container" | ||
| style={{ margin: '10px 0 0 0' }} |
There was a problem hiding this comment.
This style can be moved to sass.
| lang={lang} | ||
| color={colors.primary} | ||
| timeZone={config.timezoneData.split('|')[0]} | ||
| serviceTimeRange={context.config.itinerary.serviceTimeRange} |
There was a problem hiding this comment.
| serviceTimeRange={context.config.itinerary.serviceTimeRange} | |
| serviceTimeRange={config.itinerary.serviceTimeRange} |
| )} | ||
| </fieldset> | ||
|
|
||
| <fieldset id="timePicker"> |
There was a problem hiding this comment.
I think we should use time-picker style naming instead of timePicker for ids and class names.
| onOpen={onOpen} | ||
| onClose={onClose} | ||
| openPicker={state.open} | ||
| isHideCloseButton={state.isHideCloseButton} |
There was a problem hiding this comment.
I think better name for this parameter could be isAlwaysOpen, this is used in multiple files so rename it everywhere. The design images showed that the grey background should be removed when the timepicker is rendered from the embedded search. You should use this same prop to remove the grey background if this is true.
| const onClose = () => { | ||
| setState({ ...state, open: false }); | ||
| }; | ||
|
|
||
| const onOpen = () => { | ||
| setState({ ...state, open: true }); | ||
| }; |
There was a problem hiding this comment.
I think these can be removed if we force the timepicker to be always open in this view and open can be removed from state.
| fontWeights={config.fontWeights} | ||
| onOpen={onOpen} | ||
| onClose={onClose} | ||
| openPicker={state.open} |
There was a problem hiding this comment.
This can be hard-coded as true.
| isHideCloseButton: true, | ||
| time: undefined, | ||
| arriveBy: false, | ||
| keepPickerOpen: false, |
There was a problem hiding this comment.
I think this keepPickerOpen can be removed as well from everywhere here.
| margin: 0 !important; | ||
| padding: 0 !important; | ||
| max-width: 399px !important; |
There was a problem hiding this comment.
We should try to avoid using these !important style definitions. I think the issue here is probably that the styles from here leak to the timepicker. We should either make those generic style definitions here that affect all html elements be more specific and only affect certain classes or make some wrapper divs for the other elements in the generator and make the generic styles under those divs so they don't affect the preview.
2 participants
Proposed Changes
Pull Request Check List
Review