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

cses problem url support added #55

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

starkkumarkk1611
Copy link

I have solved issue #52
Please review my code and merge it.
If any Improvement is needed please suggest and guide me.

thanks, I would like to work on more issues like this.

@starkkumarkk1611
Copy link
Author

@Rishabh-malhotraa
Copy link
Owner

@MarufSharifi do you mind reviewing this PR and seeing if the changes make sense?

Copy link
Collaborator

@MarufSharifi MarufSharifi left a comment

Choose a reason for hiding this comment

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

Please consider a new candition in the CodeForces.tsx for cses.fi as well.

<div> {hostname === "codeforces.com" ? ( <div className={"codeforces"} ref={MathRef}> {parse(htmlString)} </div> ) : ( <div className={"atcoder"} ref={MathRef}> {parse(htmlString)} </div> )} <Divider /> </div>

Comment on lines 80 to 95
(function () {
var script = document.createElement("script");
script.type = "text/javascript";
script.src = "https://cses.fi/lib/MathJax/MathJax.js?config=TeX-AMS-MML_HTMLorMML"; // use the location of your MathJax

var config = 'MathJax.Hub.Config({' +
'extensions: ["tex2jax.js"],' +
'jax: ["input/TeX","output/HTML-CSS"]' +
'});' +
'MathJax.Hub.Startup.onload();';

script.text = config;

document.getElementsByTagName("head")[0].appendChild(script);
})();

Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove this function, because it will create a new script with every render,

Copy link
Author

Choose a reason for hiding this comment

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

Thanks, I just forgot to remove my earlier code that was not working

Comment on lines 17 to 18


Copy link
Collaborator

Choose a reason for hiding this comment

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

Please try to remove unwanted spaces

Copy link
Author

@starkkumarkk1611 starkkumarkk1611 left a comment

Choose a reason for hiding this comment

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

Thanks, I just forgot to remove my earlier javascript that was not working.
Now I have removed It and deleted unwanted white space also

@starkkumarkk1611
Copy link
Author

@Rishabh-malhotraa sorry sir for being late, I have made the requested changes. I think sir now you can merge it.

@MarufSharifi
Copy link
Collaborator

Please consider a new candition in the CodeForces.tsx for cses.fi as well.

<div> {hostname === "codeforces.com" ? ( <div className={"codeforces"} ref={MathRef}> {parse(htmlString)} </div> ) : ( <div className={"atcoder"} ref={MathRef}> {parse(htmlString)} </div> )} <Divider /> </div>

please consider this changes as well

@starkkumarkk1611
Copy link
Author

Please consider a new candition in the CodeForces.tsx for cses.fi as well.
<div> {hostname === "codeforces.com" ? ( <div className={"codeforces"} ref={MathRef}> {parse(htmlString)} </div> ) : ( <div className={"atcoder"} ref={MathRef}> {parse(htmlString)} </div> )} <Divider /> </div>

please consider these changes as well

thanks, just now I am doing sir

)}
) : ""}
{hostname === "cses.fi" ? (
<div className={"cses"} ref={MathRef}>
Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry, where does cses className come from?

Copy link
Author

Choose a reason for hiding this comment

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

if problem is of cses than div will have className="cses" as it is in "atcoder" and "codeforces"?
is that you are asking for?? i didn't get you are asking for

Copy link
Collaborator

Choose a reason for hiding this comment

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

sorry, where does cses className come from?

You didn't select cses className, you should select that and add proper style for related element

Comment on lines 32 to 45
<div className={"codeforces"} ref={MathRef}>
{parse(htmlString)}
</div>
) : (
) : ""}
{hostname === "atcoder.com" ? (
<div className={"atcoder"} ref={MathRef}>
{parse(htmlString)}
</div>
)}
) : ""}
{hostname === "cses.fi" ? (
<div className={"cses"} ref={MathRef}>
{parse(htmlString)}
</div>
) : ""}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please don't separate the conditions, they are all related to each other, for better code looking change this to as it was and add new condition

 from separate statements to a single statement
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.

None yet

3 participants