Skip to content

Dev#17

Open
sjumba wants to merge 73 commits into
masterfrom
dev
Open

Dev#17
sjumba wants to merge 73 commits into
masterfrom
dev

Conversation

@sjumba

@sjumba sjumba commented Nov 6, 2019

Copy link
Copy Markdown
Collaborator

Does display, hover , change colour and download them.

Andileh and others added 30 commits September 16, 2019 10:33
…to feature/checkBoxFunctionality

# Conflicts:
#	src/App.js
…ed again it does not take it back into original colour
Comment thread src/components/checkBoxSelection/checkBoxSelection.js Outdated
Comment thread src/components/checkBoxSelection/checkBoxSelection.js
Comment thread src/components/checkBoxSelection/checkBoxSelection.scss Outdated
Comment thread src/components/checkBoxSelection/checkBoxSelection.scss Outdated
Comment thread src/components/chooseFolder/chooseFolder.js Outdated
Comment thread src/components/displaySvg/finalSvgDisplay.js Outdated
Comment thread src/components/displaySvg/finalSvgDisplay.js Outdated
Comment thread src/components/svgSetting/svgSetting.js Outdated
Comment thread src/components/svgSetting/svgSetting.js Outdated
Comment thread src/components/svgSetting/svgSetting.js Outdated
Comment thread src/components/displaySvg/finalSvgDisplay.scss Outdated
Comment thread src/components/displaySvg/finalSvgDisplay.scss Outdated
Comment thread src/components/displaySvg/finalSvgDisplay.scss Outdated
Comment thread src/components/displaySvg/finalSvgDisplay.scss Outdated
Comment thread src/components/svgSetting/svgSetting.js Outdated
Comment thread src/components/svgSetting/svgSetting.js Outdated
Comment thread src/components/svgSetting/svgSetting.js Outdated
Comment thread src/components/checkBoxSelection/checkBoxSelection.scss Outdated
Comment thread src/components/checkBoxSelection/checkBoxSelection.js Outdated
Comment thread src/components/displayAllComponent.js Outdated
Comment thread src/components/checkBoxSelection/checkBoxSelection.js Outdated
Comment thread src/components/displayAllComponent.js Outdated
Comment thread src/components/displaySvg/finalSvgDisplay.js Outdated
Comment thread src/components/functions.js Outdated
Comment thread src/components/svgSetting/svgSetting.js Outdated
Comment thread src/components/displayAllComponent.js Outdated
let pathArrayLocal = [];
let elementIds = [];
wrapElements(SVG_TAG_NAMES, array, pathArrayLocal, 7, elementIds);
let functionStr =

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

rather inject this function directly into the SVG than inject it to the DOM and then inject it to the svg file

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I cant actually , figure out how to inject it directly into svg, as with the options I tried its either element id is undefined or cant find the function its self, and when use elem.setAttribute('onClick', function(){ // code here });

with the above approach its expose my code in the element and does nothing after all.

@hvandermerwe hvandermerwe left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

see comment

Comment thread src/components/functions.js Outdated
Comment on lines 17 to 19
export const immutablePush=(arr, newEntry)=>{
return [].concat(arr, newEntry)
}

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

if you wish to push to an array immutable you could also just say [...arr, newEntry] this will spread your existing array into a new one and add in the new entry to it

@sjumba sjumba Nov 14, 2019

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I like the approach that you have mentioned, but I am no longer using this function as its have been removed from function.js file and I will you this approach in future. thanks

Comment thread src/components/svgSetting/svgSetting.js Outdated
if (indexOf < 0) {
this.props.updateStore("svgOptions",this.props.svgOptions.concat(this.props.value))
} else {
this.props.updateStore("svgOptions",this.props.svgOptions.slice(0,indexOf).concat(this.props.svgOptions.slice(indexOf + 1))

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

so for this, for readability sake, i would say
`
let { svgOptions=[] } = this.props;
let updatedArr = indexOf < 0
? [...svgOptions, this.props.value ]
: [...svgOptions.slice(0, indexOf), ...svgOptions.slice(indexOf+1)];

this.props.updateStore("svgOptions", updatedArr );
`

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thanks, I have used spread for more readability

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

let { svgOptions=[] } = this.props; what does this line do ? what format is this, does this also fall under spread concept.

render() {
for (var key in CardLayout) {
if (CardLayout[key].type === "checkbox") {
this.state.settingOptionList.push(

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this is not how state is local state should be mutated, please make use of setState.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

While I was clearing console, I removed this loop altogether its was causing warnings regarding key duplication. components with the same key value

/>
);
} else if (CardLayout[key].type === "slider") {
this.state.settingOptionList.push(

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

this is not how state is local state should be mutated, please make use of setState.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

While I was clearing console, I removed this loop altogether its was causing warnings regarding key duplication. components with the same key value

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