Skip to content

Feature/131 add date input components #265

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

Merged
merged 17 commits into from
Feb 3, 2023

Conversation

BeneRichi
Copy link

@BeneRichi BeneRichi commented Jan 10, 2023

Pull request

I added two existing datepicker components from an other project into the boilerplate template. All necessary styles, helpers and dependencies have been installed to make those components work.

Ticket

131

Browser testing

- link

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

@patric-eberle
Copy link
Contributor

@BeneRichi should this one be reviewed or is it still work in progress?

@BeneRichi
Copy link
Author

@patric-eberle This should be reviewed.

@patric-eberle patric-eberle self-requested a review January 18, 2023 16:25
Copy link
Contributor

@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

Todo:

  • Please add the missing translations listed in the console when using the date picker on http://localhost:9090/styleguide/sandbox/forms
  • Please also add a demo for a range datepicker (by adding the range attribute to the c-date-picker element.
  • Calendar layout: please stretch or center the calendar in the available space.
  • There are exceptions in the console about missing method. Please make sure you always test with open console and do a final browser testing (with open console) before requesting a Code Review.

grafik

The rest looks good. Great work 🎉


<script>
import Pikaday from 'pikaday';
import eDate from '@/components/e-date';
Copy link
Contributor

Choose a reason for hiding this comment

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

todo: we'll have to move the e-date component to the elements folder, once that branch was merged.

@@ -0,0 +1,2 @@
@forward 'extends/button-reset';
Copy link
Contributor

Choose a reason for hiding this comment

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

note: in Vue 3 these extends can be removed, since we complete reset all browser styles there by default.

BeneRichi added 7 commits January 20, 2023 11:11
…<cDatePicker /> - rename form data properties to startDate and endDate and make them equal with today's date as default
…d height of an e-icons with the size attribute - separate nested if statements to simplify readability - rewrite comments
…" to be used inside the options object in the new VueI18n instance - call i18n.setDateTimeFormat() after fetching current page locale
…t in "const dateTimeFormats" to be used inside the options object in the new VueI18n instance - call i18n.setDateTimeFormat() after fetching current page locale
Copy link
Contributor

@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 single date field demo is not working anymore, because its value property form.date was removed.
  • The calendar allows keyboard navigation (arrow keys) but as soon as you hit the right arrow key, the value gets submitted. Can this fixed, so that only enter submits the value? (Maybe this bug is also related to the duplicated event binding I mentioned in the code.
  • The label text is always red. Can you please change the default color to a grayscale color? THX.

BeneRichi added 2 commits January 30, 2023 18:18
…fter code block - removed duplicated keypress event - changed background color of date fields from color-grayscale-800 to 600 - changed color of date input field's labels - removed color shade 800 from _color.scss - move static attribute without value after static one with value - add form.date value property to make it a directive - changed input field width

Signed-off-by: BeneRichi <[email protected]>
…color-grayscale--700 from _color.scss to reduce color classes

Signed-off-by: BeneRichi <[email protected]>
@BeneRichi
Copy link
Author

"The calendar allows keyboard navigation (arrow keys) but as soon as you hit the right arrow key, the value gets submitted. Can this fixed, so that only enter submits the value? (Maybe this bug is also related to the duplicated event binding I mentioned in the code."

@patric-eberle We have tested this again in IV and unfortunately it works the same way. Probably the calendar functionality has been extended. We need to take a close look together. Commit in IV: fde331dcb7f07e0d8cec8abbe90409a9264b9ee7

@BeneRichi BeneRichi assigned patric-eberle and unassigned BeneRichi Feb 1, 2023
@patric-eberle patric-eberle merged commit 99c87cf into vue-2 Feb 3, 2023
@patric-eberle patric-eberle deleted the feature/131_add_date_input_components branch February 3, 2023 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants