-
-
Notifications
You must be signed in to change notification settings - Fork 114
Shape and Image panels #332
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
Conversation
| break; | ||
|
|
||
| case EDITOR_ACTIONS.DELETE_IMAGE: | ||
| if (isNumeric(payload.imageIndex)) { |
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.
I guess on a later pass we could combine these delete 'annotations' 'shapes' 'images' actions into one
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.
yeah, this was a dirty copy-paste job... We'll see if it turns out that these evolve in different directions and if not, we can DRY them up.
| const key = `annotations[${annotationIndex}]`; | ||
| const value = {text: 'new text'}; | ||
| const value = {text: _('new text')}; | ||
|
|
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.
mercii
| connectImageToLayout, | ||
| connectAxesToLayout, | ||
| connectLayoutToPlot, | ||
| connectToContainer, |
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.
can we add these to the README
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.
yep
| StyleShapesPanel, | ||
| StyleImagesPanel, | ||
| StyleTracesPanel, | ||
| } from './default_panels'; |
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.
same, can we add these to the README
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.
Hmmm well none of the other default panels are in the readme, so I would vote no...
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.
I mean the positionningRef, the Accordions, the connect functions : )
| {label: _('Hide'), value: false}, | ||
| ]} | ||
| /> | ||
| <TextEditor attr="source" label={_('Source')} show /> |
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.
yeah, we need the component hein..
would you mind creating the issue so we don't forget..
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.
yep. we can use react-dropzone I think, I used it for https://react-pivottable.js.org/ and it works great.
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.
|
PR feedback addressed! |
|
💃 |
Roughed in, lots of fit'n'finish needed :(