Skip to content

Conversation

@matheusbsilva137
Copy link
Contributor

Proposed changes (including videos or screenshots)

  • Update nivo version (which was causing errors in the bar chart);
  • Fix 1 day delay in '7 days' and '30 days' periods;
  • Update tooltip theme.

Issue(s)

Steps to test or reproduce

Access Administration > Engagement Dashboard.
Expected behavior:
new-users-engagement-dashboard

Further comments

import GenericTable from './GenericTable';
import HeaderCell from './HeaderCell';

export default Object.assign(GenericTable, {
Copy link
Member

Choose a reason for hiding this comment

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

Curious as to why you used Object.assign here 🤓 Is it just a shortcut to writing GenericTable.HeaderCell = HeaderCell; in the line above? Or is there another reason?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh boy, you are the right guy to ask me this one. GenericTable has a function type (kind of (props: any) => ReactElement) defined at './GenericTable'. It doesn't provide any mechanism to extend it while changing the concrete value: GenericTable.HeaderCell = HeaderCell ; would trigger an error for me because GenericTable's type is not an extension of { HeaderCell: (props: any) => ReactElement; }. I've noticed that Object.assign() intersects the types of its arguments, so we get { (props: any): ReactElement; HeaderCell: (props: any) => ReactElement; } at the return type.

Copy link
Member

Choose a reason for hiding this comment

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

Got it!

Does this generate a side-effect? I mean, if someone imported from the ./GenericTable file directly, the GenericTable exported there would also have the HeaderCell object due to Object.assign?

Of course, that wouldn't be a problem; TS wouldn't consider the original object to contain HeaderCell and I guess even the usual console.log would not print it there, it could only be confusing if you tried to see GenericTable's keys with Object.keys or other similar functions. Just asking, again, out of curiosity 😛

@matheusbsilva137 matheusbsilva137 merged commit f475e6b into develop Jul 27, 2021
@matheusbsilva137 matheusbsilva137 deleted the regression/new-users-ed branch July 27, 2021 20:10
gabriellsh added a commit that referenced this pull request Jul 27, 2021
…ile_upload

* 'develop' of github.com:RocketChat/Rocket.Chat:
  Regression: Data in the "New Users" section is delayed in 1 day (#22751)
  [FIX] Support ID param on createVisitor method (#22772)
  Regression: fix non ee tag field on canned responses (#22775)
  [FIX] Blank screen in message auditing DM tab (#22763)
  Show translated scope on cr dashboard (#22773)
  Regression: Fix empty tag field (#22767)
  Regression: Federation warnings on ci (#22765)
  Regression: fix outdated data on canned filters (#22766)
  [IMPROVE] Return open room if available for visitors (#22742)
  Regression: Fix empty canned responses table when searching (#22743)
  Regression: Filter of canned responses in contextual-bar (#22762)
  Regression: Fix users not being able to see the scope of the canned message (#22760)
  Regression: Fixes empty department field on edit canned responses (#22741)
  Regression: Allow users to update canned responses scope (#22738)
  allow users to search canned responses based on shortcut or content (#22735)
  Regression: Check for text before parse preview in create canned response form (#22754)
@ggazzo ggazzo mentioned this pull request Jul 31, 2021
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.

4 participants