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

The Update function had some problems. Now it is fine. #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

MKM12345
Copy link

No description provided.

@hamdivazim
Copy link
Owner

Before I merge this pull request, could you explain what you have fixed?

@hamdivazim hamdivazim self-assigned this Apr 10, 2023
@MKM12345
Copy link
Author

MKM12345 commented Apr 12, 2023

codingboy_CW I have not fixed everything but I have changed one thing.

Not all CSS properties could be added and sometimes the changes couldn't come live and I changed the update() into what’s in the script.js now so that it does.

I’m not sure about the body element though. Run my code to see if it’s fixed but I’m not sure. The problem was that The body{} was changing the entire HTML content as well as the test-div.

@hamdivazim
Copy link
Owner

I've tested the code and unfortunately it is now broken. If you could fix it so that changes are made as you type, that would be great.

Note of advice: only open pull requests when you are sure what you have added works :)

@MKM12345
Copy link
Author

MKM12345 commented Apr 13, 2023

Unfortunately the same is happening with yours.

I will update the code again, but originally it hasn't been working.
Watch the video below to see.

Uploading codingboy_CW - Live CSS Editor! and 2 more pages - Work - Microsoft​ Edge 2023-04-13 19-13-25.mp4…

@hamdivazim
Copy link
Owner

Really? Strange ... it seems to be working for me and others - check your system to make sure everything is fine.

@MKM12345
Copy link
Author

Actually, I tested it on many different devices, same thing.

@hamdivazim
Copy link
Owner

Can you describe exactly what the issue you're facing is? And how you've fixed that in this PR too - that will help me debug the issue.

@hamdivazim
Copy link
Owner

Also, which devices did you test on?

@MKM12345
Copy link
Author

MKM12345 commented Apr 14, 2023

Confidential - sorry, but I can tell you some devices:
Iphone 13 pro max
HP Pavilion Laptop Convertible
Galaxy Samsung Note 9
Macbook Pro 16"
Also what error are you getting in the javascript in the Pull request? It works for me.

@MKM12345
Copy link
Author

The live CSS editor; the changes aren't coming live.

Sometimes it works with many bugs.

Also the body{} changes the whole HTML content, instead of the .test-div.

@hamdivazim
Copy link
Owner

The body { } is purposefully there to modify the whole website, and .test-div { } is there to just modify the div. Also, please specify which of the 'many bugs' you are talking about so I can look into things and fix the issue. The live system should work though; could you explain how often the changes take effect?

@MKM12345
Copy link
Author

For me - never.

@hamdivazim
Copy link
Owner

Ok, since the issue seems to only be on your side and this PR breaks it fully, I can't merge this pull request until the issues on your side are fixed. Sorry 😔

@MKM12345
Copy link
Author

What? This pr is fully functional, check again - you may have put something wrong

@MKM12345
Copy link
Author

And by the way - no, its not just for me. Im getting issues with multiple devices, different places, checked all the settings, scanned the network, and everything was fine. It has to be an issue with this.

@hamdivazim
Copy link
Owner

And by the way - no, its not just for me. Im getting issues with multiple devices, different places, checked all the settings, scanned the network, and everything was fine. It has to be an issue with this.

I meant with other people. Also please send a video showing the issue you're getting on the website

@hamdivazim
Copy link
Owner

Your PR seems to be working - can you send a video of the website not working so I can see how this pull request fixes the issue

@MKM12345
Copy link
Author

And by the way - no, its not just for me. Im getting issues with multiple devices, different places, checked all the settings, scanned the network, and everything was fine. It has to be an issue with this.

I meant with other people. Also please send a video showing the issue you're getting on the website

Yeah, thats also what Im saying.

@hamdivazim
Copy link
Owner

Can I see the video?

@MKM12345
Copy link
Author

Wait a sec

@MKM12345
Copy link
Author

The file size max limit is too small-sorry(10MB)

@hamdivazim
Copy link
Owner

Email me at codingboy.cw@gmail.com

@MKM12345
Copy link
Author

Please check your inbox

@hamdivazim
Copy link
Owner

Do you have any logs at all to shed some light as to what is going on? Try checking Chrome DevTools with F12 to see if you spot anything at all.

@MKM12345
Copy link
Author

image
All I could get, sorry

@MKM12345
Copy link
Author

Do you have any logs at all to shed some light as to what is going on? Try checking Chrome DevTools with F12 to see if you spot anything at all.

Maybe my solution is better

@MKM12345
Copy link
Author

Do you have any logs at all to shed some light as to what is going on? Try checking Chrome DevTools with F12 to see if you spot anything at all.

Maybe my solution is better

This was my plan:
The Live CSS Editor code had a couple of issues that prevented the CSS changes from being applied live on the page. Here are the issues and the changes I made to fix them:

The way CSS was being applied:
The original code was using the innerHTML property to add the CSS styles to the page, which replaced the entire HTML content of the element, causing the changes not to appear live.

I tried using the style property of the DOM elements to directly modify their CSS styles, which allowed me to modify the styles of individual elements on the page without replacing the entire content of the style element.

The update() function:
The original update() function was replacing the entire content of the style element with the value of the css-editor-textarea element, which was not an ideal approach because it caused the changes not to appear live.

I made the update() function to apply the CSS changes to individual elements on the page rather than replacing the entire style element. I also added a check to create a style element if one did not exist.

@hamdivazim
Copy link
Owner

Right ok, I'll add these changes in - thanks!

@MKM12345
Copy link
Author

So are you going to merge the pr

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.

2 participants