Add "file sources" to Iodide #1985
Conversation
…rontend file source api in place
…ds, UX tweaks before actual design
There was a problem hiding this comment.
-
in the files sources menu i'm noticing something weird happening at certain widths and certain heights-- some sides of the borders or outlines around the windows and buttons disappear. I cannot reproduce on chrome, so if you have not seen this either perhaps we can chalk it up to a rendering bug on FF for linux. to attempt to reproduce you can use the "responsive design mode" in FF devtools at e.g 1452x1191 (just one example)
-
let's be sure to add lint rules for hooks before merging this, see: https://github.com/airbnb/javascript/tree/master/packages/eslint-config-airbnb and https://reactjs.org/docs/hooks-rules.html#eslint-plugin
-
@hamilton @wlach -- re: "as far as I know, the file update operation statuses have not changed. This is a backend operation that will require a migration. I haven't done this part yet, so the labels will appear the same as they did before" -- this makes it sound like the labels are just being passed straight through from the server? not a blocker, but this feels like a tighter coupling between the UI and the API than is probably desirable. Semantically, something like "update operation status" sounds like be an enum that we should maps to UI strings so that changes like this are trivial and only require touching the UI code.
-
@hamilton the transitions are a nice touch, and add a lot to the legibility of delete actions, but as you noted, they add a lot of noise to our code. for simple enter/exit animations in lists, i think react-transition-group might nicely cover this need?
https://reactcommunity.org/react-transition-group/transition-group
https://github.com/reactjs/react-transition-group
It seems like it deals with basic transitions in a pretty clean way (and perhaps other simple animation needs), and handing off the animation logic could simplify our code
| url: { type: ["string", "null"] }, | ||
| filename: { type: "string" }, | ||
| update_interval: { type: ["string", "null"] }, | ||
| last_file_update_operation: fileUpdateOperationSchema |
There was a problem hiding this comment.
hmm weird, when i search-all-files in ./src for the string "last_file_update_operation" i can't find it anywhere other than here. which component uses it?
src/editor/components/modals/file-modal/manage-file-sources/file-source-list.jsx
Outdated
Show resolved
Hide resolved
src/editor/components/modals/file-modal/manage-file-sources/file-source-list.jsx
Outdated
Show resolved
Hide resolved
src/editor/components/modals/file-modal/manage-file-sources/file-source-list.jsx
Outdated
Show resolved
Hide resolved
src/editor/components/modals/file-modal/manage-file-sources/__tests__/validators.test.js
Outdated
Show resolved
Hide resolved
src/editor/components/modals/file-modal/manage-file-sources/validators.js
Outdated
Show resolved
Hide resolved
…; last_file_update_operation -> latest_file_update_operation in schema
|
In testing on definitely-staging, hovering over the user icon produces a weird effect:
|
server/files/api_views.py
Outdated
| return base.filter(id__in=filter_by_id) | ||
| return base | ||
|
|
||
| queryset = FileSource.objects.all() |
There was a problem hiding this comment.
Is queryset and get_queryset needed here?
server/files/models.py
Outdated
| verbose_name = "File Update Operation" | ||
| verbose_name_plural = "File Update Operations" | ||
| ordering = ("id",) | ||
| db_table = "file_operation" |
There was a problem hiding this comment.
shouldn't this be called file_update_operation to stay consistent?
wlach
dismissed
bcolloran’s stale review
Hamilton addressed feedback, can always circle back to further comments later
Pull Request checklist