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

Calculator example uses outdated nodegui API #4

Open
TekuConcept opened this issue Jan 21, 2020 · 4 comments
Open

Calculator example uses outdated nodegui API #4

TekuConcept opened this issue Jan 21, 2020 · 4 comments

Comments

@TekuConcept
Copy link

@TekuConcept TekuConcept commented Jan 21, 2020

Cloning this repo, building, and running as-is works perfectly thanks to the project-lock.json. (I believe this might be why the issue was overlooked.) But running npm install @nodegui/nodegui within a new project and copying over the example source files shows missing or differing API elements.

The example project is using @nodegui/nodegui v0.6.5
The latest version at this time is @nodegui/nodegui v0.12.1

  • QPushButtonEvents has been renamed to QPushButtonSignals; This object also seems to be treated differently than before, so the snipped QPushButtonSignals.clicked will not work.
  • BaseWidgetEvents has been removed.

I would advise amending the project.json file so as to limit support to that last compatible version of the package. (running npm update on this example will break it)

@srsholmes

This comment has been minimized.

Copy link

@srsholmes srsholmes commented Jan 22, 2020

I had the same issue, I think hooks are the way to use the events now, this might help.

e.g

const btnHandler = useEventHandler<QPushButtonSignals>(
    {
      clicked: () => open("https://react.nodegui.org").catch(console.log)
    },
    []
  );

as shown in the starter kit here....

https://github.com/nodegui/react-nodegui-starter/blob/master/src/components/steptwo.tsx#L7

I may be able to do a PR to update the examples to use the new versions if i get time.

@matrunchyk

This comment has been minimized.

Copy link

@matrunchyk matrunchyk commented Feb 8, 2020

I have the same issue with the react-router-example right after updating it to a newest API versions of node-gui and react-gui. To reproduce it, I you can do the following:

  1. clone the example
  2. change the versions so they look as follows:
"@nodegui/nodegui": "^0.13.4",
"@nodegui/react-nodegui": "^0.4.0",
  1. Change the event handlers in About and Home files so they look as follows:
const handler = useEventHandler<QPushButtonSignals>(
  { clicked: () => history.push('/about') },
  [],
);
  1. Run the example, click "Go to About", "Go to Home"
  2. Expected: it shows "Go to About" again.
  3. Actual: it shows empty page and logs "MainWindow can't have more than one child node" warning.

As a result, I cannot use router with the newest API (I don't want to start my application using the old one).

Also, it looks like the documentation is not up to date too.

@master-atul

This comment has been minimized.

Copy link
Collaborator

@master-atul master-atul commented Feb 9, 2020

Hi @matrunchyk I have fixed the example code.

I had to add an extra View component .

The extra View is needed since React's reconciler tries to add a view before removing a child view.
We can fix this in the next release of react-nodegui. Till then we will need to use the extra View component.

You can find the changes needed in this commit here: e8bff26

@matrunchyk

This comment has been minimized.

Copy link

@matrunchyk matrunchyk commented Feb 9, 2020

Cool! Thank you very much! For now the extra View will work great for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.