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.

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

Fix file disappearing in userData field #1908

Merged
merged 11 commits into from
Jan 30, 2024
Merged

Conversation

benjaminleonard
Copy link
Contributor

Fixes #1907

For reasons outlines in that issue, CSS animation had become troublesome.

Here we keep the accordion mounted so never lose the file state.

Will defer to you all on whether this feels like a better fix than controlling the file input.

Copy link

vercel bot commented Jan 24, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
console ✅ Ready (Inspect) Visit Preview Jan 29, 2024 11:40pm

@benjaminleonard benjaminleonard marked this pull request as draft January 24, 2024 12:17
@benjaminleonard
Copy link
Contributor Author

This has climbed in complexity a little, which might be a case for use controlling the file input. Perhaps the other question here is whether we might have some other input state that becomes more awkward after demounting and remounting.

@benjaminleonard
Copy link
Contributor Author

Will defer to y'all on the best way to tackle this – this PR or a controlled input.

If we want a quick uncontroversial fix I can just disable the animation and add forceMount and we can revisit in the next release.

@benjaminleonard benjaminleonard marked this pull request as ready for review January 26, 2024 09:52
control: Control<InstanceCreateInput>
isSubmitting: boolean
}) => {
const [value, setValue] = useState<string[]>([])
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does it still need this advancedness to solve the original problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Advanced is the name of the accordion. This was just a general refactor to make it easier and neater to add to this section in the future.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm talking about the useState, which wasn't there before.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My bad, you're right I can do this all with css

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

...or can I. We need isOpen to do the auto scrolling when you open an accordion I believe.

This reverts commit 4d08e86.
@david-crespo
Copy link
Collaborator

I feel good about this fix as-is.

I do think there's a good chance controlling the file input would be better, as you say — it feels like the right structure from the form's POV. Without this fix, the form state value does stay set — the problem is about initializing the state of the file input when in pops into existence as the accordion expands.

A counterpoint is that we still need to track isOpen in order to trigger scrollIntoView when the thing expands. I'll give the controlled input thing a try, and if it feels bad I'll just merge this since it's good to go.

@david-crespo david-crespo merged commit 764f731 into main Jan 30, 2024
8 checks passed
@david-crespo david-crespo deleted the user-data-accordion-fix branch January 30, 2024 03:08
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Jan 31, 2024
oxidecomputer/console@b9013a3...1a4f5d8

* [1a4f5d81](oxidecomputer/console@1a4f5d81) oxidecomputer/console#1910
* [34596e33](oxidecomputer/console@34596e33) oxidecomputer/console#1928
* [ff7cdb28](oxidecomputer/console@ff7cdb28) oxidecomputer/console#1926
* [efa39789](oxidecomputer/console@efa39789) oxidecomputer/console#1867
* [4bfadc02](oxidecomputer/console@4bfadc02) oxidecomputer/console#1927
* [695d3671](oxidecomputer/console@695d3671) oxidecomputer/console#1925
* [30070292](oxidecomputer/console@30070292) better path filter for local files in msw handler
* [5cf4339c](oxidecomputer/console@5cf4339c) oxidecomputer/console#1916
* [a26f7c1e](oxidecomputer/console@a26f7c1e) oxidecomputer/console#1922
* [231b93ed](oxidecomputer/console@231b93ed) oxidecomputer/console#1923
* [c8364638](oxidecomputer/console@c8364638) better msw warning filter so we don't get warning noise in console
* [764f7310](oxidecomputer/console@764f7310) oxidecomputer/console#1908
* [945619eb](oxidecomputer/console@945619eb) oxidecomputer/console#1921
* [e2d82a4c](oxidecomputer/console@e2d82a4c) oxidecomputer/console#1887
* [d6a67bd5](oxidecomputer/console@d6a67bd5) oxidecomputer/console#1918
* [1fb746f4](oxidecomputer/console@1fb746f4) oxidecomputer/console#1899
* [ca7c85de](oxidecomputer/console@ca7c85de) oxidecomputer/console#1917
* [28598e1d](oxidecomputer/console@28598e1d) oxidecomputer/console#1914
* [34eb478d](oxidecomputer/console@34eb478d) oxidecomputer/console#1912
* [4d693088](oxidecomputer/console@4d693088) bump vite-plugin-html to stop getting deprecation warning
* [d5c39549](oxidecomputer/console@d5c39549) oxidecomputer/console#1909
* [7c6f53db](oxidecomputer/console@7c6f53db) oxidecomputer/console#1854
david-crespo added a commit to oxidecomputer/omicron that referenced this pull request Jan 31, 2024
oxidecomputer/console@b9013a3...1a4f5d8

* [1a4f5d81](oxidecomputer/console@1a4f5d81)
oxidecomputer/console#1910
* [34596e33](oxidecomputer/console@34596e33)
oxidecomputer/console#1928
* [ff7cdb28](oxidecomputer/console@ff7cdb28)
oxidecomputer/console#1926
* [efa39789](oxidecomputer/console@efa39789)
oxidecomputer/console#1867
* [4bfadc02](oxidecomputer/console@4bfadc02)
oxidecomputer/console#1927
* [695d3671](oxidecomputer/console@695d3671)
oxidecomputer/console#1925
* [30070292](oxidecomputer/console@30070292)
better path filter for local files in msw handler
* [5cf4339c](oxidecomputer/console@5cf4339c)
oxidecomputer/console#1916
* [a26f7c1e](oxidecomputer/console@a26f7c1e)
oxidecomputer/console#1922
* [231b93ed](oxidecomputer/console@231b93ed)
oxidecomputer/console#1923
* [c8364638](oxidecomputer/console@c8364638)
better msw warning filter so we don't get warning noise in console
* [764f7310](oxidecomputer/console@764f7310)
oxidecomputer/console#1908
* [945619eb](oxidecomputer/console@945619eb)
oxidecomputer/console#1921
* [e2d82a4c](oxidecomputer/console@e2d82a4c)
oxidecomputer/console#1887
* [d6a67bd5](oxidecomputer/console@d6a67bd5)
oxidecomputer/console#1918
* [1fb746f4](oxidecomputer/console@1fb746f4)
oxidecomputer/console#1899
* [ca7c85de](oxidecomputer/console@ca7c85de)
oxidecomputer/console#1917
* [28598e1d](oxidecomputer/console@28598e1d)
oxidecomputer/console#1914
* [34eb478d](oxidecomputer/console@34eb478d)
oxidecomputer/console#1912
* [4d693088](oxidecomputer/console@4d693088)
bump vite-plugin-html to stop getting deprecation warning
* [d5c39549](oxidecomputer/console@d5c39549)
oxidecomputer/console#1909
* [7c6f53db](oxidecomputer/console@7c6f53db)
oxidecomputer/console#1854
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

userData uploaded file disappears when accordion is closed
2 participants