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

2024 Crescendo VictiScout Update #211

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

2024 Crescendo VictiScout Update #211

wants to merge 5 commits into from

Conversation

1ceit
Copy link
Member

@1ceit 1ceit commented Jan 30, 2024

  • Update to conform to game
    - Add Type of drive
    - Add Chasis Size (updateable in table view)
    - Add Height (updateable in table view)
    - Add Climb Method dropdown
    - Add Auto Starting position dropdown
    - Add Note Scoring

  • Add Shot Accuracy
    - Add Auto Shot Accuracy field in table view
    - Add Tele-Op Shot Accuracy field in table view
    - Add Sort By Auto Shot Accuracy button in table view
    - Add Sort By Tele-Op Shot Accuracy button in table view

  • Add Export to CSV button

  • Clean Up UI
    - Hide Input file
    - Hide Convert file
    - Hide input file textbox
    (Hides 10,000 px off the screen, breaks if deleted)

    Fix Just use CSV by default instead of JSON #178, Checkboxes are off center #170

@1ceit 1ceit added the annual label Jan 30, 2024
@1ceit 1ceit requested review from ErikBoesen, MyaTaheri and BenBerol and removed request for BenBerol January 30, 2024 15:43
MyaTaheri
MyaTaheri previously approved these changes Jan 30, 2024
css/data.css Outdated
}
#transfer-button {
margin-left: 20px;
}
#HideTheseButtonsCuzItBreakIfIDelete{
Copy link
Member

Choose a reason for hiding this comment

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

I would much prefer that you figure out why it's breaking and then resolve the breakage rather than keeping dead code around. Happy to help out if you give further clarification on what is going wrong when you remove the buttons in question.

css/data.css Outdated Show resolved Hide resolved
css/data.css Outdated Show resolved Hide resolved
css/data.css Outdated Show resolved Hide resolved
css/data.css Outdated Show resolved Hide resolved
// Define <thead>, <tbody>, warning, and match deletion vars to be filled later on.
var thead = document.getElementsByTagName('thead')[0],
tbody = document.getElementsByTagName('tbody')[0],
warning = document.getElementById('warning'),

processingSection = document.getElementById('processing'),
fileInputButton = document.getElementById('input-file'),
Copy link
Member

Choose a reason for hiding this comment

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

Oh, this is why it's breaking if you delete that HTML code, because you're still trying to get the elements in the JS code

Copy link
Member Author

Choose a reason for hiding this comment

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

I am aware of the JS get element. there are functions with in that are called that use the elements. I Spent about an hour trying to get it to work... I just was tired and hid it.

js/data.js Outdated Show resolved Hide resolved
js/script.js Outdated Show resolved Hide resolved
@1ceit
Copy link
Member Author

1ceit commented Jan 31, 2024

Anything I haven't resolved still needs to be fixed give me a bit and ill get it done :)

@ErikBoesen
Copy link
Member

I'm still a little hung up on the hidden inputs and I would prefer that this question gets resolved before merging.

Do we not want to have support for merging and transferring files anymore? I would question this decision as I recall those being pretty fundamental features of the app and the primary reason for the data screen existing in the first place. But if we don't want them anymore, then we should remove them outright including the fairly substantial logic that manages them.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Just use CSV by default instead of JSON
3 participants