Calendar integration ui #3395
Calendar integration ui #3395
Conversation
| @@ -93,7 +93,7 @@ class NavigateSectionList extends Component<Props> { | |||
|
|
|||
| return ( | |||
| <SectionList | |||
| ListEmptyComponent = { renderListEmptyComponent } | |||
| ListEmptyComponent = { renderListEmptyComponent() } | |||
There was a problem hiding this comment.
Was this change necessary? SectionList doc says ListEmptyComponent can be a render function.
| @@ -6,17 +6,32 @@ import Container from './Container'; | |||
| import Text from './Text'; | |||
| import type { Item } from '../../Types'; | |||
|
|
|||
| type Props = { | |||
| export type Props = { | |||
There was a problem hiding this comment.
Is the export necessary? I may be missing where it's being used outside of this file.
| label: t('welcomepage.calendar'), | ||
| content: CalendarList ? <CalendarList /> : null, | ||
| defaultSelected: true | ||
| }, |
There was a problem hiding this comment.
Atlaskit tabs automatically hides the tab if it has no content?
| @@ -1,3 +1,3 @@ | |||
| export { default as ConferenceNotification } from './ConferenceNotification'; | |||
| export { default as MeetingList } from './MeetingList'; | |||
| export { default as CalendarList } from './CalendarList'; | |||
There was a problem hiding this comment.
Gotta remove MeetingList.web.js.
| @@ -0,0 +1,126 @@ | |||
| // @flow | |||
There was a problem hiding this comment.
Was this file a copy/paste from MeetingList with no changes?
| const { disabled } = this.props; | ||
|
|
||
| return ( | ||
| AbstractCalendarList |
There was a problem hiding this comment.
Aren't abstract classes normally extended?
| = createSection(t('calendarSync.today'), TODAY_SECTION); | ||
| const sectionMap = new Map(); | ||
|
|
||
| sectionMap.set(TODAY_SECTION, todaySection); |
There was a problem hiding this comment.
I think this means sections will always have a length of at least 1, which means SectionList will never show ListEmptyComponent.
| }) } | ||
| </Container> | ||
| ) | ||
| ) |
There was a problem hiding this comment.
A loading icon somewhere would be nice. It can take 2+ seconds for the calendar data to load.
| @@ -68,6 +85,7 @@ export default class NavigateSectionListItem extends Component<Props> { | |||
| className = 'navigate-section-tile-body'> | |||
| { duration } | |||
| </Text> | |||
There was a problem hiding this comment.
Is NavigateSectionListItem specifically for calendar entries or do you know if it was intended to be something more general? It's markup makes it look like it was specific for calendar entries.
| @@ -68,6 +85,7 @@ export default class NavigateSectionListItem extends Component<Props> { | |||
| className = 'navigate-section-tile-body'> | |||
| { duration } | |||
There was a problem hiding this comment.
Do you know much about the data here and why an array is used with assumption about what each index in the array stands for? It looks like index 0 might be meeting url but the variable name is date. Index 1 might be data but the variable name is duration.
| @@ -93,7 +93,7 @@ class NavigateSectionList extends Component<Props> { | |||
|
|
|||
| return ( | |||
| <SectionList | |||
| ListEmptyComponent = { renderListEmptyComponent() } | |||
There was a problem hiding this comment.
This was fixing an issue and was intentional. With the change below the case of disconnected calendar doesn't work anymore, so I reverted this change.
The problem basically is that NavigateSectionList expects a function, while SectionList expects a Component.
There was a problem hiding this comment.
I have a fix I'm working on for it now, which should try to fix the empty component never displaying. It seems like it worked on native because it was calling _getRenderListEmptyComponent() in the calendarlist instead of passing in _getRenderListEmptyComponent.
| { duration } | ||
| </Text> | ||
| </Container> | ||
| { elementAfter || null } |
There was a problem hiding this comment.
If we add the elementAfter, as a completely constructed element, then we don't need the action props I've added previously. I'm going to remove them.
No description provided.