Skip to content
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

Bugfix/fix invisible logs #746

Closed
wants to merge 3 commits into from

Conversation

rotdrop
Copy link
Contributor

@rotdrop rotdrop commented Jun 28, 2022

See issue #699

This pull request just replaces react-addons-css-transition-group with react-transition-group, following the guide here:

https://github.com/reactjs/react-transition-group/blob/HEAD/Migration.md

For me this seems to fix #699.

Notes: I have problem with the package.json of the logreader pacakge, it specifies

engines": {
                "node": "^14.0.0",
                "npm": "^7.0.0"
        },

However. to my knowledge node v14 comes with npm v6, while npm v7 comes only with node v15. But then we have here conflicting versions of the required engines. As the package-lock in the repo already is V2, I would suspect that the the node-version is just wrong.

@rotdrop rotdrop force-pushed the bugfix/fix-invisible-logs branch from 2681655 to 881d86e Compare June 28, 2022 11:42
@rotdrop rotdrop force-pushed the bugfix/fix-invisible-logs branch from 881d86e to 7ff83bf Compare August 17, 2022 08:27
@rotdrop rotdrop mentioned this pull request Aug 17, 2022
<ReactCSSTransition
key={i}
classNames="highlight"
timeout={{ enter: 15000, exit: 1500 }}
Copy link

Choose a reason for hiding this comment

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

As a sanity check (I don't know much about React): you add 15000 vs 1500, and below it was 1500 and 1500. Is that intended?

@bencomp
Copy link

bencomp commented Aug 27, 2022

One of the actions failed, apparently because the package acorn could not be found. Is that something to look into?

…p with react-transition-group

Just tried to follow the migration guide:

https://github.com/reactjs/react-transition-group/blob/HEAD/Migration.md

This commit addresses the issue that the invisibl log-entries
experienced with Nextcloud v24 are caused by pulling in conflicting
version of react-dom. See issue nextcloud#699.

Signed-off-by: Claus-Justus Heine <[email protected]>
Signed-off-by: Claus-Justus Heine <[email protected]>
Signed-off-by: Claus-Justus Heine <[email protected]>
@rotdrop rotdrop force-pushed the bugfix/fix-invisible-logs branch from 7ff83bf to 22ad1b5 Compare November 9, 2022 10:59
@icewind1991
Copy link
Member

icewind1991 commented Nov 24, 2022

Thanks for the fix!

I've cherry picked the relevant commit into a bigger PR (#821) to reduce the rebasing/conflict resolving needed

@icewind1991 icewind1991 mentioned this pull request Nov 24, 2022
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.

3 participants