-
Notifications
You must be signed in to change notification settings - Fork 6.8k
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
Calendar navigation #24124
Calendar navigation #24124
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,6 +31,8 @@ node_modules | |
/.vs | ||
*.swo | ||
*.swp | ||
.vimrc | ||
.nvimrc | ||
|
||
# misc | ||
.DS_Store | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,7 +31,12 @@ $calendar-range-end-body-cell-size: | |
padding-right: $calendar-body-label-side-padding; | ||
} | ||
|
||
.mat-calendar-body-cell-wrapper { | ||
display: contents; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is this necessary? Also it seems this isn't supported in Safari. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, well, the MDN Compatibility table for display says there is some issues on Safari with unusual elements 🤔 . I tried this on Safari and it seems to render fine. Safari Version 15.1 (17612.2.9.1.20) |
||
} | ||
|
||
.mat-calendar-body-cell { | ||
display: table-cell; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We had an issue where NVDA would announce an extra "table" because something had |
||
position: relative; | ||
height: 0; | ||
line-height: 0; | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -122,6 +122,9 @@ export class MatCalendarBody implements OnChanges, OnDestroy { | |||
/** Emits when a new value is selected. */ | ||||
@Output() readonly selectedValueChange = new EventEmitter<MatCalendarUserEvent<number>>(); | ||||
|
||||
/** Emits when a new date becomes active. */ | ||||
@Output() readonly activeValueChange = new EventEmitter<MatCalendarUserEvent<number>>(); | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What is this event used for? It seems like we're only emitting it/forwarding it. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, good question. What this is trying to accomplish is making it so that the calendar body can update the value of the This might be easier to do with banana in a box rather than passing an event. That is currently (in master branch) how the
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ok, so if I'm understanding it correctly, what it's trying to achieve is to sync the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Exactly, if I had more time, I would even go as far as renaming |
||||
|
||||
/** Emits when the preview has changed as a result of a user action. */ | ||||
@Output() readonly previewChange = new EventEmitter< | ||||
MatCalendarUserEvent<MatCalendarCell | null> | ||||
|
@@ -153,6 +156,13 @@ export class MatCalendarBody implements OnChanges, OnDestroy { | |||
} | ||||
} | ||||
|
||||
/** Called when a cell is focused. */ | ||||
_cellFocused(cell: MatCalendarCell, event: any): void { | ||||
if (cell.enabled) { | ||||
this.activeValueChange.emit({value: cell.value, event}); | ||||
} | ||||
} | ||||
|
||||
/** Returns whether a cell should be marked as selected. */ | ||||
_isSelected(value: number) { | ||||
return this.startValue === value || this.endValue === value; | ||||
|
@@ -337,7 +347,11 @@ export class MatCalendarBody implements OnChanges, OnDestroy { | |||
// Only reset the preview end value when leaving cells. This looks better, because | ||||
// we have a gap between the cells and the rows and we don't want to remove the | ||||
// range just for it to show up again when the user moves a few pixels to the side. | ||||
if (event.target && isTableCell(event.target as HTMLElement)) { | ||||
if ( | ||||
event.target && | ||||
(event.target as HTMLElement).parentElement && | ||||
isTableCell((event.target as HTMLElement).parentElement as HTMLElement) | ||||
) { | ||||
this._ngZone.run(() => this.previewChange.emit({value: null, event})); | ||||
} | ||||
} | ||||
|
@@ -350,7 +364,7 @@ export class MatCalendarBody implements OnChanges, OnDestroy { | |||
if (isTableCell(element)) { | ||||
cell = element; | ||||
} else if (isTableCell(element.parentNode!)) { | ||||
cell = element.parentNode as HTMLElement; | ||||
cell = element; | ||||
} | ||||
|
||||
if (cell) { | ||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -149,13 +149,14 @@ describe('MatMonthView', () => { | |
fixture.detectChanges(); | ||
}); | ||
|
||
it('should decrement date on left arrow press', () => { | ||
fit('should decrement date on left arrow press', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leftover from debugging? |
||
dispatchKeyboardEvent(calendarBodyEl, 'keydown', LEFT_ARROW); | ||
fixture.detectChanges(); | ||
expect(calendarInstance.date).toEqual(new Date(2017, JAN, 4)); | ||
|
||
calendarInstance.date = new Date(2017, JAN, 1); | ||
fixture.detectChanges(); | ||
expect(calendarInstance.date).toEqual(new Date(2017, JAN, 1)); | ||
|
||
dispatchKeyboardEvent(calendarBodyEl, 'keydown', LEFT_ARROW); | ||
fixture.detectChanges(); | ||
|
@@ -653,7 +654,16 @@ describe('MatMonthView', () => { | |
(_userSelection)="userSelectionSpy($event)"></mat-month-view>`, | ||
}) | ||
class StandardMonthView { | ||
date = new Date(2017, JAN, 5); | ||
get date(): Date { | ||
console.log('test code. `get date`', this._date); | ||
return this._date; | ||
} | ||
set date(date: Date) { | ||
console.log('test code. `set date`', date); | ||
this._date = date; | ||
} | ||
_date = new Date(2017, JAN, 5); | ||
|
||
selected: Date | DateRange<Date> = new Date(2017, JAN, 10); | ||
selectedChangeSpy = jasmine.createSpy('selectedChange'); | ||
userSelectionSpy = jasmine.createSpy('userSelection'); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why don't we use a
button
element instead of adiv
with arole
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the draft implementation of it, it was the path of least resistance to use
div role="button"
because it doesn't affect the visual styles. We would need to override a few (not sure exactly how many) style rules to get abutton
element to look correct.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's only 5-6 lines of CSS so it might be easier to do it now https://github.com/angular/components/blob/master/src/material/core/style/_button-common.scss#L5.