-
Notifications
You must be signed in to change notification settings - Fork 1
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
feat: add new schedule table #13
Conversation
|
||
const timeRenderer = (timeZone: string) => (value: string) => | ||
value ? moment(value, 'YYYY-MM-DD HH:mmZ').tz(timeZone).format('HH:mm') : ''; | ||
|
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.
Let's replace these two functions with a more generic one:
const renderDate = (timeZone: string, format: string) => (value: string) =>
value ? moment(value, 'YYYY-MM-DD HH:mmZ').tz(timeZone).format(format) : '';
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.
or consider some of the methods from client/src/components/Table/renderers.tsx
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.
done
// })} | ||
columns={[ | ||
{ | ||
title: columnFilter(), |
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 will look more readable if we left <SettingOutlined />
here. The function that contains filter
in the name and defines the title is a bit confusing...
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.
fix
width: 100, | ||
dataIndex: ['event', 'type'], | ||
render: (value: keyof typeof EventTypeColor) => ( | ||
<Tag color={EventTypeColor[value]}>{EventTypeToName[value] || value}</Tag> |
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.
Let's use renderTag
method here. It's declared in client/src/components/Table/renderers.tsx
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.
renderTag doesn't fit format, create new one in client/src/components/Table/renderers.tsx - eventColorTagRenderer
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 can't we do renderTag(EventTypeColor[value], EventTypeToName[value] || value)
?
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.
fix
render: (value: string, record) => { | ||
return record.event.descriptionUrl ? ( | ||
<a target="_blank" href={record.event.descriptionUrl}> | ||
{value} |
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.
Let's move it to it's own function
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.
simplify render in one line
return url ? ( | ||
<Tooltip placement="topLeft" title={url}> | ||
<a target="_blank" href={url}> | ||
{urlRenderer(url)} |
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.
There is no need for ternary operator. The construction can be simplified:
url => !!url && <Tooltip ...
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.
fix
title: 'Organizer', | ||
width: 140, | ||
dataIndex: ['organizer', 'githubId'], | ||
render: (value: string) => (value ? <GithubUserLink value={value} /> : ''), |
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.
The same here and above. There is no need in ternary operator
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.
fix
title: 'Place', | ||
dataIndex: 'place', | ||
render: (value: string) => { | ||
return value === 'Youtube Live' ? ( |
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.
let's move the render to it's own function
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.
done
// }, | ||
// })} | ||
columns={[ | ||
{ |
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.
WDYT about moving the columns to it's own constant?
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.
the columns depends on props (timeZone)...
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 can be function... It won't improve performance, but will improve readability at least
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.
done
// onClick: ({ target }) => { | ||
// if (!target.closest('a')) { | ||
// // eslint-disable-next-line no-console | ||
// console.log('row is clicked'); |
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.
Shouldn't this be uncommented?
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.
remove comments
* feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers
* feat: add new schedule table (#13) * feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers * feat: add switcher schedule view mode * feat: add user settings * chore: refactor code - exclude switch * chore: refactor code * chore: refactor code - add uppercase * fix: implement correct localstorage hook * feat: add userSettings button on schedule page * feat: add user settings * refactor: change serialize/deserialize data * refactor: rename files and data * refactor: change tag color styles structure * refactor: rename vars for better readability * refactor: apply memoization to setTagColor * refactor: change structure of TagIcon * refactor: replace settings button, tweak button styles, change storedTagColors type Co-authored-by: notSAINT <60464016+not-SAINT@users.noreply.github.com> Co-authored-by: Aleksandr Sharikov <notsaint.mail@gmail.com>
* feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers
* feat: add new schedule table (#13) * feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers * feat: add switcher schedule view mode * feat: add user settings * chore: refactor code - exclude switch * chore: refactor code * chore: refactor code - add uppercase * fix: implement correct localstorage hook * feat: add userSettings button on schedule page * feat: add user settings * refactor: change serialize/deserialize data * refactor: rename files and data * refactor: change tag color styles structure * refactor: rename vars for better readability * refactor: apply memoization to setTagColor * refactor: change structure of TagIcon * refactor: replace settings button, tweak button styles, change storedTagColors type Co-authored-by: notSAINT <60464016+not-SAINT@users.noreply.github.com> Co-authored-by: Aleksandr Sharikov <notsaint.mail@gmail.com>
* feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers
* feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers
* feat: add new schedule table (#13) * feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers * feat: add switcher schedule view mode * feat: add user settings * chore: refactor code - exclude switch * chore: refactor code * chore: refactor code - add uppercase * fix: implement correct localstorage hook * feat: add userSettings button on schedule page * feat: add user settings * refactor: change serialize/deserialize data * refactor: rename files and data * refactor: change tag color styles structure * refactor: rename vars for better readability * refactor: apply memoization to setTagColor * refactor: change structure of TagIcon * refactor: replace settings button, tweak button styles, change storedTagColors type Co-authored-by: notSAINT <60464016+not-SAINT@users.noreply.github.com> Co-authored-by: Aleksandr Sharikov <notsaint.mail@gmail.com>
* feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers
* feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers
* feat: add new schedule table (#13) * feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers * feat: add switcher schedule view mode * feat: add user settings * chore: refactor code - exclude switch * chore: refactor code * chore: refactor code - add uppercase * fix: implement correct localstorage hook * feat: add userSettings button on schedule page * feat: add user settings * refactor: change serialize/deserialize data * refactor: rename files and data * refactor: change tag color styles structure * refactor: rename vars for better readability * refactor: apply memoization to setTagColor * refactor: change structure of TagIcon * refactor: replace settings button, tweak button styles, change storedTagColors type Co-authored-by: notSAINT <60464016+not-SAINT@users.noreply.github.com> Co-authored-by: Aleksandr Sharikov <notsaint.mail@gmail.com>
* feat: add new schedule table * chore: refactor cells renderers * chore: refactor renderers
Добавлен базовый вид расписания в виде таблицы.
Типа событий не трогались.