-
-
Notifications
You must be signed in to change notification settings - Fork 7
Open brush integration #20
base: main
Are you sure you want to change the base?
Conversation
andybak
commented
Nov 21, 2023
- Initial simple search functionality
- Remove category search placeholder
- Thumbnail view/upload support for assets
- Generate a device code to enable login from within Open Brush
- Update the poly assets browser to use poly.pizza instead via our wrapper API (Not part of the main nav - more of a test at the moment. Might hide this entirely)
Not sure how useful it is but I wanted to test the api
@@ -3,3 +3,5 @@ node_modules/ | |||
.env | |||
.env.dev | |||
.env.live | |||
build | |||
.idea/* |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nitpick, but .idea/* and other IDE related config ignores should be in your local global gitignore file IMO. Happy to leave this, but most places I've worked only have gitignore ignore files that might be generated/required by the app specifically.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fair point. It's good practice and is probably something I should be doing anyway.
Sorry I've taken a while to get around to this. First thing I've noticed, getting SSL errors due to a security vulnerability fixed in node 17. See solution. I can run it by downgrading my node version, but we probably should consider whether we want to upgrade to compatibility with the latest LTS instead. |
I haven't had a chance to properly poke at the code yet, but search doesn't appear to work at all for me. Is it searching by name, description, ID? It looks like it's calling the API with the search, are there API updates too or is the one at https://api.icosa.gallery/ on the latest? |
Yep. There's matching changes on a branch for the back-end too. I've been running it locally but I guess we could deploy it. @mikeskydev ? |
Getting this building took me almost as long as writing the code. I get completely lost with node and package version issues (which is quite a statement coming from a Python dev!) so I was wary of changing anything. Is this issue a blocker? What's the impact? |
It’s not a blocker per se, though newer versions of node will refuse to run it so we’ll have to stick with 16. I wouldn’t say it would block the merge but it is an issue to be addressed soon for certain. |