-
Notifications
You must be signed in to change notification settings - Fork 370
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
[frontend]Storage Browser initial layout implementation #3397
Conversation
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.
Nice work, few comments.
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.
Nice work. It is complex getting this setup but this looks good.
@@ -17,6 +17,9 @@ export async function loadComponent(name) { | |||
case 'FileChooserWithButton': | |||
return (await import('./FileChooser/FileChooserWithButton/FileChooserWithButton')).default; | |||
|
|||
case 'StorageBrowserPage': |
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.
nit: Move up to "Page specific components here" section
@@ -520,6 +521,9 @@ def listdir_paged(request, path): | |||
filter=? - Specify a substring filter to search for in | |||
the filename field. | |||
""" | |||
if ENABLE_NEW_STORAGE_BROWSER.get(): | |||
return render('storage_browser.mako', request, {}) |
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.
Nice, this is needed to get the new react app started, but additional routing within the new storage browser should probably be handed by a browser based router like the react router. Lets discuss once we get there.
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.
Nice work!
Fix the interface import statement and then good to go!
@@ -75,6 +75,14 @@ | |||
database: string; | |||
} | |||
|
|||
interface BrowserInterpreter { |
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.
Add to imports above from config/types.ts (if needed)
08f9777
to
2bd263a
Compare
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.
LGTM
* [frontend]Storage Browser initial layout implementation * [frontend]Fixed python styling * [frontend]Changed feature flag to enable_new_storage_browser * [frontend]Moved models.py changes to UI and improvements * [frontend]Added data-browser icon and improvements (cherry picked from commit 0c47195)
…3397) * [frontend]Storage Browser initial layout implementation * [frontend]Fixed python styling * [frontend]Changed feature flag to enable_new_storage_browser * [frontend]Moved models.py changes to UI and improvements * [frontend]Added data-browser icon and improvements (cherry picked from commit 0c47195) (cherry picked from commit bc07c22) Change-Id: I10899ac6c6d7d92844d400fb3303eb9d9351b6e5 (cherry picked from commit 5afbef3)
…3397) * [frontend]Storage Browser initial layout implementation * [frontend]Fixed python styling * [frontend]Changed feature flag to enable_new_storage_browser * [frontend]Moved models.py changes to UI and improvements * [frontend]Added data-browser icon and improvements (cherry picked from commit 0c47195) (cherry picked from commit bc07c22) Change-Id: I10899ac6c6d7d92844d400fb3303eb9d9351b6e5
What changes were proposed in this pull request?
How was this patch tested?
Please review Hue Contributing Guide before opening a pull request.