2377: Show versions in the UI#2690
Conversation
|
|
||
| <script> | ||
| $(document).ready(() => { | ||
| $('#version_date_select').change((event) => { |
There was a problem hiding this comment.
I often use data- attributes to handle JS because I think ids often get used to also handling styling. It feels like the data- attributes are more aligned with adding JS functionality.
What do you think? Could we name this to something data-version-data-select or something?
There was a problem hiding this comment.
Hmm... I see data- attributes more as handling... well, data. You basically inject some kind of information into a DOM element that can then be pulled into JavaScript. I don't think they are well-suited to actually finding the element you're going to be manipulating using JS.
There was a problem hiding this comment.
Hmm... I see data- attributes more as handling... well, data.
That makes sense :P. I guess I perceive data as including something like JS since JS interacts with those data-. I believe stimulus uses these attributes to bind JS behaviors onto DOM elements. This is where I'am drawing that connection.
I think a bit of more context on why I'am suggesting this can help clarify my suggestion. Suppose you have a case where we add CSS via the id and also bind to some JS function, and a situation arises where you want the JS functionality but not the CSS in some cases. This is kind of a contrived example but it happened to me in the past ;).
In summary, I'am not going to be a zealot and force one way or the other (in this case ;)). Interested to hear your thoughts on this.
There was a problem hiding this comment.
for part 1 - the reason Stimulus uses data attributes is simply that it doesn't know anything about the application, since it's a library. :) So it needs to either use classes, or data, and in this case data is more appropriate.
for part 2 - it's definitely something that could happen. We basically can decide to use ID's for CSS and data for JavaScript, or IDs for JavaScript and classes for CSS. It's not a huge difference but I do feel that data is best used for actual data and not for querying the DOM.
There was a problem hiding this comment.
Both points make sense! Thanks for providing that insight.
I think there is no clear standard right now and I don't think its worth holding up this PR for that reason :). Would be interesting to work with you to see if we can bring standards in!
edwinthinks
left a comment
edwinthinks
left a comment
There was a problem hiding this comment.
I think this PR is looking good! I just wanted to pick your brain on a few things and see where we get. Afterwards, I'am happy to approve :)
|
@edwinthinks updated! |
edwinthinks
left a comment
edwinthinks
left a comment
There was a problem hiding this comment.
The stakeholders are going to be sooo happy about this! This has been a desired feature for ages!
|
Yahoo! |
2 participants
Resolves #2377
Description
This adds a date select on the Storage Location page which allows the user to select a date to show the inventory at. If there was no inventory at that date, it will show "N/A".
Type of change
How Has This Been Tested?
Local and unit testing. Would like to see some more real data.
Screenshots