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

[CLOSED] Replaced toggleClass with add/removeClass where appropriate #3457

Open
core-ai-bot opened this issue Aug 29, 2021 · 8 comments
Open

Comments

@core-ai-bot
Copy link
Member

Issue by WebsiteDeveloper
Wednesday May 01, 2013 at 07:04 GMT
Originally opened as adobe/brackets#3689


Partial fix for #3338


WebsiteDeveloper included the following code: https://github.com/adobe/brackets/pull/3689/commits

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Wednesday May 01, 2013 at 21:47 GMT


It looks like this is implementing 'option A' from #3338, which is the most verbose of all the approaches. Any interest in looking into the cleaner options B or C?

@core-ai-bot
Copy link
Member Author

Comment by WebsiteDeveloper
Thursday May 02, 2013 at 05:09 GMT


i did look at B and C. For me B seems the best for compatibility but i thought if this is decided a little later we could first merge this and maybe after that still change those places to use an utility function.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Thursday May 02, 2013 at 20:45 GMT


I think it would be a lot easier to do that cleanup now, rather than going through later and trying to identify addClass()/removeClass() pairs to migrate over.

@core-ai-bot
Copy link
Member Author

Comment by WebsiteDeveloper
Friday May 03, 2013 at 05:03 GMT


You are right, actually it would be fairly easy because all show up in this requests diff, but if you think it's better then i'll just add an utility function.

@core-ai-bot
Copy link
Member Author

Comment by WebsiteDeveloper
Friday May 03, 2013 at 19:08 GMT


@peterflynn i added an util function, so i basically implemented option B.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Friday May 10, 2013 at 22:17 GMT


@WebsiteDeveloper done reviewing

@core-ai-bot
Copy link
Member Author

Comment by WebsiteDeveloper
Saturday May 11, 2013 at 14:22 GMT


@peterflynn fixes pushed.

@core-ai-bot
Copy link
Member Author

Comment by peterflynn
Monday May 13, 2013 at 23:55 GMT


Thanks@WebsiteDeveloper! Merging now.

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

No branches or pull requests

1 participant