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

Open issue from pop-up goes to openworm #205

Open
dosumis opened this issue Sep 12, 2019 · 4 comments
Open

Open issue from pop-up goes to openworm #205

dosumis opened this issue Sep 12, 2019 · 4 comments
Assignees
Labels

Comments

@dosumis
Copy link
Member

dosumis commented Sep 12, 2019

image

=> issue on openworm/org.geppetto

Should go here - incorporating log.

@ddelpiano ddelpiano self-assigned this Sep 24, 2019
@ddelpiano
Copy link

Open issue voice on the bottom point to openworm at the moment, to be changed to point to the VFB.

@ddelpiano
Copy link

ddelpiano commented Oct 16, 2019

@jrmartin the issue sits in geppetto-client/js/components/controls/modals/ErrorModal.js, in order to fix this we need to be able to inject 2 props:

Since we need to do this for the "Open issue" button I would the same also for the tweet button "Shame on you" so the user can decide to enable/disable both the button and configure them.

A way I thought to do this could be to re-use the GeppettoConfiguration.json (or istantiate another configuration) where we have something similar:

{
....
  errorDialog: {
    twitterButton: {
      enabled: true
      url: 'https://twitter.com/openworm'
      message: 'I broke geppetto, ops!'
    }
  }
}

So the same structure can be used also for the other modals in a second moment.
The error modal will try to get this info from the GeppettoConfiguration (or similar) but if this is not available it has a default configuration in the component itself to use if this is not provided (the geppetto one).
I would go for something similar like this if @tarelli it's ok.

@tarelli
Copy link

tarelli commented Oct 17, 2019

Sounds reasonable, I wouldn't add a new configuration though. And currently everything is flat within the GeppettoConfiguration but this isn't necessarily a problem. Create a card if it's not there already in geppett-client for this addition. Once you guys have the final proposed change to the file put it there for review, we can also discuss at the next Geppetto call.

@jrmartin
Copy link

First proposed changes to make this happen are done, waiting on review of PR openworm/geppetto-client#111.

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

No branches or pull requests

4 participants