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

1804: Adding hint for birth date input for koblenz #1858

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
+72 −29
Open

1804: Adding hint for birth date input for koblenz #1858

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cfc96cc
1804: adding hint for birth date input
bahaaTuffaha Jan 14, 2025
cb02c2e
1804: blueprintjs fix multiple imports
bahaaTuffaha Jan 14, 2025
c8c1620
1804: Adding showBirthdayMinorHint flag to each project config
bahaaTuffaha Jan 14, 2025
2f557f2
Merge remote-tracking branch 'origin' into 1804-Adding-hint-button-fo…
bahaaTuffaha Jan 14, 2025
ff37657
1804: Fixing the mock issue for csvExport test and removing a duplica…
bahaaTuffaha Jan 15, 2025
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
60 changes: 35 additions & 25 deletions administration/src/bp-modules/components/CustomDatePicker.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,14 @@
import { DesktopDatePicker } from '@mui/x-date-pickers'
import { deDE } from '@mui/x-date-pickers/locales'
import React, { ReactElement } from 'react'
import styled from 'styled-components'

import useWindowDimensions from '../../hooks/useWindowDimensions'

const StyledDiv = styled.div`
display: flex;
align-items: center;
`
type CustomDatePickerProps = {
date: Date | null
onBlur: () => void
Expand All @@ -13,6 +18,7 @@ type CustomDatePickerProps = {
minDate?: Date
maxDate?: Date
disableFuture: boolean
sideComponent?: ReactElement
}

const CustomDatePicker = ({
Expand All @@ -24,38 +30,42 @@ const CustomDatePicker = ({
minDate,
maxDate,
disableFuture,
sideComponent,
}: CustomDatePickerProps): ReactElement => {
const { viewportSmall } = useWindowDimensions()
const formStyle = viewportSmall ? { fontSize: 16, padding: '9px 10px' } : { fontSize: 14, padding: '6px 10px' }
const textFieldBoxShadow =
'0 0 0 0 rgba(205, 66, 70, 0), 0 0 0 0 rgba(205, 66, 70, 0), inset 0 0 0 1px #cd4246, inset 0 0 0 1px rgba(17, 20, 24, 0.2), inset 0 1px 1px rgba(17, 20, 24, 0.3)'
return (
<DesktopDatePicker
views={['year', 'month', 'day']}
value={date}
format='dd.MM.yyyy'
slotProps={{
clearIcon: { fontSize: viewportSmall ? 'medium' : 'small' },
openPickerIcon: { fontSize: 'small' },
field: { clearable: true, onClear },
textField: {
placeholder: 'TT.MM.JJJJ',
error: !isValid,
spellCheck: false,
onBlur,
sx: {
width: '100%',
input: formStyle,
boxShadow: !isValid ? textFieldBoxShadow : undefined,
<StyledDiv>
<DesktopDatePicker
views={['year', 'month', 'day']}
value={date}
format='dd.MM.yyyy'
slotProps={{
clearIcon: { fontSize: viewportSmall ? 'medium' : 'small' },
openPickerIcon: { fontSize: 'small' },
field: { clearable: true, onClear },
textField: {
placeholder: 'TT.MM.JJJJ',
error: !isValid,
spellCheck: false,
onBlur,
sx: {
width: '100%',
input: formStyle,
boxShadow: !isValid ? textFieldBoxShadow : undefined,
},
},
},
}}
localeText={deDE.components.MuiLocalizationProvider.defaultProps.localeText}
disableFuture={disableFuture}
minDate={minDate}
maxDate={maxDate}
onChange={onChange}
/>
}}
localeText={deDE.components.MuiLocalizationProvider.defaultProps.localeText}
disableFuture={disableFuture}
minDate={minDate}
maxDate={maxDate}
onChange={onChange}
/>
{sideComponent}
Copy link
Member

Choose a reason for hiding this comment

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

The icon is sadly not correctly vertically centered/aligned:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

looks like the parent is not flex

Copy link
Contributor

Choose a reason for hiding this comment

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

but first check where we want to put it in the end (see other comment)

</StyledDiv>
)
}

Expand Down
30 changes: 28 additions & 2 deletions administration/src/cards/extensions/BirthdayExtension.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,24 @@
import { FormGroup } from '@blueprintjs/core'
import React, { ReactElement, useState } from 'react'
import { Classes, FormGroup, Tooltip } from '@blueprintjs/core'
import HelpOutlineIcon from '@mui/icons-material/HelpOutline'
import React, { ReactElement, useContext, useState } from 'react'
import styled from 'styled-components'

import CustomDatePicker from '../../bp-modules/components/CustomDatePicker'
import FormErrorMessage from '../../bp-modules/self-service/components/FormErrorMessage'
import { ProjectConfigContext } from '../../project-configs/ProjectConfigContext'
import PlainDate from '../../util/PlainDate'
import { Extension, ExtensionComponentProps } from './extensions'

const StyledToolTip = styled(Tooltip)`
border: 0;
width: 20px;
margin-left: 16px;
`

const StyledHelpOutlineIcon = styled(HelpOutlineIcon)`
color: #595959;
`

export const BIRTHDAY_EXTENSION_NAME = 'birthday'
export type BirthdayExtensionState = { [BIRTHDAY_EXTENSION_NAME]: PlainDate | null }

Expand All @@ -21,6 +34,8 @@ const BirthdayForm = ({
const [touched, setTouched] = useState(false)
const { birthday } = value
const showErrorMessage = touched || showRequired
const projectConfig = useContext(ProjectConfigContext)

const getErrorMessage = (): string | null => {
if (!birthday) {
return 'Bitte geben Sie ein gültiges Geburtsdatum an.'
Expand All @@ -45,6 +60,17 @@ const BirthdayForm = ({
isValid={isValid || !showErrorMessage}
maxDate={new Date()}
disableFuture
sideComponent={
<>
{projectConfig.showBirthdayMinorHint && (
<StyledToolTip
className={Classes.TOOLTIP_INDICATOR}
content='Bei Minderjährigen unter 16 Jahren darf der KoblenzPass nur mit Einverständnis der Erziehungsberechtigten abgerufen werden.'>
Copy link
Member

Choose a reason for hiding this comment

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

@f1sh1918 how is it with translations? Should we move this there? Or even to some build config specific place as this is koblenz-only?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes it should be in the translations file. we may create a namespaces "extensions" for this.
Currently i think we have no need to create project specific translations in administration but it could be necessary for the future. I wouldn't put effort in this currently but maybe create overrides in the future

<StyledHelpOutlineIcon fontSize='small' />
</StyledToolTip>
)}
</>
}
Comment on lines +63 to +73
Copy link
Member

Choose a reason for hiding this comment

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

🤔 I think we should not pass this to the DatePicker component, this is not a thing it should be concerned with. Instead, we should make sure the styling is correct here and render the hint directly here.
I see three options:

  1. Render it next to the label of the FormGroup (label accepts also React nodes)
  2. Use FormGroups helperText prop and just display a text, possibly only if the birthday is actually underage
  3. Render it here next to the date input and fix the styling here. I don't really like it though as it makes the inputs have different lengths

@f1sh1918 what do you think?

Copy link
Contributor

@f1sh1918 f1sh1918 Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Hm i think you should ask @hauf-toni
We have already an explanation to the "Aktenzeichen" form field which opens a modal with explanations
i'm not pretty sure if we want to implement the same behavior but it makes it easier for the user to have sth. consistent and predictable

image

Copy link
Contributor

@seluianova seluianova Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Just wanted to share that I also liked it more, when all fields have the same width.

As an idea, what if we show this warning as a dialog after the form submission, only if the selected date was under 18? And the user could explicitly confirm that he is "einverstanden".. 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

i would even ask toni

/>
{showErrorMessage && <FormErrorMessage errorMessage={getErrorMessage()} />}
</FormGroup>
Expand Down
1 change: 0 additions & 1 deletion administration/src/cards/extensions/extensions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ const Extensions = [
AddressPlzExtension,
AddressLocationExtension,
NuernbergPassIdExtension,
NuernbergPassIdExtension,
steffenkleinle marked this conversation as resolved.
Show resolved Hide resolved
EMailNotificationExtension,
KoblenzReferenceNumberExtension,
] as const
Expand Down
1 change: 1 addition & 0 deletions administration/src/project-configs/bayern/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,7 @@ const config: ProjectConfig = {
enabled: false,
},
userImportApiEnabled: false,
showBirthdayMinorHint: false,
}

export default config
1 change: 1 addition & 0 deletions administration/src/project-configs/getProjectConfig.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ export type ProjectConfig = {
selfServiceEnabled: boolean
storesManagement: StoresManagementConfig
userImportApiEnabled: boolean
showBirthdayMinorHint: boolean
}

export const setProjectConfigOverride = (hostname: string): void => {
Expand Down
1 change: 1 addition & 0 deletions administration/src/project-configs/koblenz/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const config: ProjectConfig = {
selfServiceEnabled: true,
storesManagement: storesManagementConfig,
userImportApiEnabled: true,
showBirthdayMinorHint: true,
Copy link
Member

Choose a reason for hiding this comment

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

I think this would make it clearer, other wise it sounds like a minor (as in not that important) hint

Suggested change
showBirthdayMinorHint: true,
showUnderageHint: true,

Copy link
Contributor

@seluianova seluianova Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

showBirthdayInfoHint or showBirthdayHint, if the text is customizable per project? (it could contain any kind of info then?)

Copy link
Contributor

@f1sh1918 f1sh1918 Jan 20, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

hm since we have no real configuration structure for our extensions i think we may include the extension name in the flag. Maybe we want to refactor the extensions structure in the future and this flag can move then to the extension itself

}

export default config
5 changes: 4 additions & 1 deletion administration/src/project-configs/nuernberg/__tests__/csvExport.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ jest.mock('csv-stringify/browser/esm/sync', () => ({
stringify: (input: string[][]) => input[0].join(','),
}))

jest.mock('../../getProjectConfig')
jest.mock('../../getProjectConfig', () => ({
__esModule: true,
default: jest.fn(),
}))

describe('csvExport', () => {
it('header should have same length as line', () => {
Expand Down
1 change: 1 addition & 0 deletions administration/src/project-configs/nuernberg/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@ const config: ProjectConfig = {
selfServiceEnabled: false,
storesManagement: storesManagementConfig,
userImportApiEnabled: false,
showBirthdayMinorHint: false,
}

export default config
1 change: 1 addition & 0 deletions administration/src/project-configs/showcase/config.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ const config: ProjectConfig = {
enabled: false,
},
userImportApiEnabled: false,
showBirthdayMinorHint: false,
}

export default config