Skip to content

Fixes Multiple TODO's in Code #4673

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
wants to merge 4 commits into from
Closed

Fixes Multiple TODO's in Code #4673

wants to merge 4 commits into from

Conversation

Govi2020
Copy link

@Govi2020 Govi2020 commented Apr 9, 2025

Description

All major changes made were made according to TODO's that was in the comments in the code.

  • pages/options.js --> Now if there is a error then the window will scroll into view to show the error and input in validation
  • content_scripts/mode_visual --> has unneeded closure, which was changed into plain for loop
  • content_scripts/linkhints.js --> css was hard coded for popups which was changed just adding a classname
  • content_scripts/vimium.css -> moved the hard coded css to this file as recommend by the dev

Auto Format code was a pain in the neck since i am using astro vim.Formatting created a lot of unnecessary changes which you can ignore and you can focus your attention on the actual changes that was made.
I have tried my best to follow the contributing guidelines . But I am new to contributing to open source projects , so i would be very grateful if you would point out the problems in my code for me to fix. which i will get working on immediately.

@Govi2020 Govi2020 changed the title Dev Fixes Multiple TODO's in Code Apr 9, 2025
@philc
Copy link
Owner

philc commented Apr 10, 2025

Hi @Govi2020, thanks for looking into these. There is too much noise in the diffs for me to find the changes you're referring to, and it makes this difficult to merge. You can run deno fmt which will format the code base according to our style rules.

Second, probably at least some of these should be broken out into their own PRs so they're easier to review and merge, rather than bundled together.

@Govi2020
Copy link
Author

I have addressed the problems and did a push request #4678

@Govi2020 Govi2020 closed this Apr 10, 2025
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