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

QoL improvements for code, updated battery display code, and more #57

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

Conversation

ironswordX-dev
Copy link

@ironswordX-dev ironswordX-dev commented Nov 28, 2023

  • Improve battery display message (QOL) #35 by @Ant-Throw-Pology was the baseline for updated battery display code, I turned most of the needed code to generate the time left into its own function that automatically gives outputs depending on inputs (e.g. if the function is given 0 as the hours left and 15 as the minutes left, it will give Around 15 minutes left until (direction) rather than Around 0 hours 15 minutes until (direction))
  • Improved function importing for the new tab javascript files using the default tag in export statements
  • File that contains all the urls needed (still working on)
  • Added a link w/ a small instruction dialogue to add/connect/etc. wifi networks using chrome://network/#select
  • And a few other miscellaneous things

@bypassiwastaken
Copy link
Owner

most of this seems okay. but a few nitpicks:

  • wifiselect.js is not a valid file name, it should be wifi-select.js
  • the URL thing should not be called "URL" with all uppercase, that conflicts with the actual api built in
  • I'd prefer if you named that object UrlConstants, export defaulted it, then did "import UrlConstants from 'url-constants.js'", then referenced those as UrlConstants.HISTORY_URL
  • Just to recap, "wifiLink" html/css class should be "wifi-link". "wifilink" js variable should be "wifiLink"
  • Also what is "import { openTab } from "./../wifiselect.js";"??? i dont see this file anywhere

If you do all these i'll merge

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