Add Image select dropdown for Device Images#23
Closed
hartmans wants to merge 3 commits into
Closed
Conversation
Provide a dropdown for device selection rather than requiring users to enter the image ID. This implementation is architecturally impure in that it introduces dependencies between the devices feature and the images feature. Fixing this requires a more central model store in the frontend.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This is AI generated code. I think we're going to need to make a number of usability improvements in a short period of time so I'm okay with that.
It took about 20 minutes to generate, and that's probably the turn around time we want to be looking at right now. But it definitely is worth a pass to improve css styling if necessary.
I do notice one architectural problem with the approach. It introduces a dependency between features/devices and features/images, which we probably do not want.
Long term we want something on the frontend like a model store so that the UI parts of the frontend can access data models for other aspects to do this sort of cross-concern usability.
I don't think we have time to build that before we start asking people at Hackathon2 to use the code.
If you accept this code (or code with a similar defficiency), open a bug on the architectural issue, as well as any other temporary deficiencies you want to clean up later.