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.

Sign up for GitHub

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

Jump to bottom

Model render refactor #5894

Merged
merged 18 commits into from
Nov 10, 2023
Merged

Model render refactor #5894

merged 18 commits into from
Nov 10, 2023

Conversation

SchrodingersGat
Copy link
Member

Removes duplicate model rendering code in the new react interface

@SchrodingersGat SchrodingersGat added user interface User interface refactor Platform UI Related to the React based User Interface labels Nov 9, 2023
@netlify Netlify
Copy link

netlify bot commented Nov 9, 2023 •
edited

✅ Deploy Preview for inventree-web-pui-preview ready!

Name Link
🔨 Latest commit ac4f770
🔍 Latest deploy log https://app.netlify.com/sites/inventree-web-pui-preview/deploys/654db123b40ba400088ed68f
😎 Deploy Preview https://deploy-preview-5894--inventree-web-pui-preview.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
Lighthouse
Lighthouse		 1 paths audited
Performance: 100 (no change from production)
Accessibility: 86 (no change from production)
Best Practices: 92 (no change from production)
SEO: 70 (no change from production)
PWA: -
View the detailed breakdown and full score reports

To edit notification comments on pull requests, go to your Netlify site configuration.

@SchrodingersGat
"Fix" scanning page
9d92e87 
- Rendering is a bit broken currently, as the barcode scan does not send back the model data
@SchrodingersGat
"Fix" scanning page
6f33738 
- Rendering is a bit broken currently, as the barcode scan does not send back the model data
- Required refactoring enumerations out into separate files
- Some strange race condition / import loop was happening
@SchrodingersGat
Merge remote-tracking branch 'inventree/master' into model-render-ref…
f810fe7 
…actor

# Conflicts:
#	src/frontend/src/components/tables/plugin/PluginListTable.tsx
@SchrodingersGat
Fix incorrect imports
f0cb94d
@SchrodingersGat
Fixing hover card
d7d3543 
- Use unique key
@SchrodingersGat
fixes
c9b301e
@SchrodingersGat
Fix urls.md
37ad810
@SchrodingersGat
More udpates
524925e
@SchrodingersGat
Fix unused import
ac4f770
@SchrodingersGat SchrodingersGat added this to the 0.13.0 milestone Nov 10, 2023
@SchrodingersGat SchrodingersGat merged commit e6db817 into inventree:master Nov 10, 2023
23 checks passed
@SchrodingersGat SchrodingersGat deleted the model-render-refactor branch November 10, 2023 04:44
@matmair
Copy link
Contributor

matmair commented Nov 10, 2023

To me it seems we lose functionality with this refactor. My renderers had the option to link to the object they were rendering while yours were just text/images. This greatly reduces the clicks needed to get to a specific object - I think that should be reintroduced.

@SchrodingersGat
Copy link
Member Author

@matmair happy to add that in. But not happy for two ways to render a single model! If you think this has rescued functionality please make a PR :)

@matmair
Copy link
Contributor

matmair commented Nov 10, 2023

Yeah most of my WIP UI stuff seems to break after this, I will have to read the diff a bit deeper an reintroduce a few things

@wolflu05
Copy link
Contributor

Yes, the api changes will also break my WIP things.

@matmair
Copy link
Contributor

matmair commented Nov 13, 2023

this removed so much of the interactivenes of renderes, I will revert this for my code base and work with the old variant for now to meet my deadlines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Platform UI Related to the React based User Interface refactor user interface User interface
Projects
None yet
Milestone
0.13.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@SchrodingersGat @matmair @wolflu05