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

Feature/321 Migrates e-date and c-date-picker component #344

Merged
merged 6 commits into from
Jan 16, 2024

Conversation

BeneRichi
Copy link

@BeneRichi BeneRichi commented Jan 5, 2024

Pull request

This PR brings two new features to the form:

  1. e-date component: Enables users to enter a single date.
  2. c-date-picker component (Picker): Allows users to pick a date within a calendar overlay.
  3. c-date-picker component (Range Picker): Allows users to pick a range between two dates within a calendar overlay.

I've also included new translation strings into de.json + a new dependency package called pikaday .

Before testing, please run npm install or npm ci to ensure that the package is installed.

Ticket

http://localhost:5173/styleguide/sandbox/forms

Browser testing

#321

Checklist

  • I merged the current development branch (before testing)
  • Added JSDoc and styleguide demo
  • Tested all links in project relevant browsers
  • Tested all links in different screen sizes
  • Did run automated tests and linters
  • Did assign ticket
  • Double checked target branch

Review/Test checklist

  • Did review code and documentation
  • Tested all links in project relevant browsers
  • Tested all links in different screen sizes
  • Did check accessibility (Wave, only errors)
  • Re-assign ticket to developer

…into r-forms.

- Adds pikaday dependency and type definition.
@BeneRichi BeneRichi changed the base branch from master to develop January 5, 2024 15:06
@BeneRichi BeneRichi self-assigned this Jan 5, 2024
@BeneRichi BeneRichi added enhancement New feature or request components New components to add or enhancements on exiting components labels Jan 5, 2024
Copy link
Member

@patric-eberle patric-eberle left a comment

Choose a reason for hiding this comment

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

Browser review

  • The calendar icon can not be used to interact with the field. Nothing happens if you click on the icon.
  • I would recommend to remove the calendar icon from the "date only" (without calendar popover) field.
  • When the calendar is shown, the field content also gets the black background. I would recommend to kepp the background white.
  • It is not possible to change to a different month.
  • Even though the calendar value changes, the displayed field value never does.
  • There is no spacing between the calendar title and the close icon (on mobile):
grafik

Optional

  • It seems not to be possible to enter single digit dates (without a leading 0). e.g. 1.1.2024.
  • When opening and closing the calendar again and then running a WAVE test, the column headers are missing in the table. I don't think this would be an easy fix... But Probably also not a real problem, since the calendar will not be shown to the visitor in this case anyway.

src/components/c-date-picker.vue Outdated Show resolved Hide resolved
src/components/c-date-picker.vue Outdated Show resolved Hide resolved
src/elements/e-date.vue Outdated Show resolved Hide resolved
src/elements/e-date.vue Outdated Show resolved Hide resolved
src/elements/e-date.vue Show resolved Hide resolved
src/plugins/dayjs.ts Outdated Show resolved Hide resolved
src/setup/scss/variables/_z-index.scss Outdated Show resolved Hide resolved
src/styleguide/routes/r-forms.vue Outdated Show resolved Hide resolved
BeneRichi and others added 3 commits January 11, 2024 14:05
- Re-Orders z-index variables
- Changes root div element to span in e-date
- Adds nextThick in mounted hook in c-date-picker
# Conflicts:
#	src/compositions/form-states.ts
@patric-eberle
Copy link
Member

@BeneRichi the browser review issues are still open.

@BeneRichi
Copy link
Author

@BeneRichi the browser review issues are still open.

I know. I would have let you know as soon as I had made all the changes.

BeneRichi added 2 commits January 12, 2024 14:49
- Adds slot within e-date.
- Adds  pointer-events: none to e-input for the e-icon.
@patric-eberle patric-eberle merged commit ab0f96c into develop Jan 16, 2024
@patric-eberle patric-eberle deleted the feature/321_Migrate_e-date_c-date-picker branch January 16, 2024 10:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
components New components to add or enhancements on exiting components enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants