Skip to content

fix(site): Fix template version editor rename #6251

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

Merged
merged 13 commits into from
Mar 6, 2023

Conversation

BrunoQuaresma
Copy link
Collaborator

No description provided.

Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

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

Looking good! I tested out renaming and deleting but when I rename the old file stays and when I delete it closes the editor but the file is still there.

tree

<Stack>
<p>
Rename <strong>{filename}</strong> to something else. This path can
contain slashes too!
Copy link
Member

Choose a reason for hiding this comment

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

I see above we changed ! to ., should we do that here too?

@BrunoQuaresma
Copy link
Collaborator Author

BrunoQuaresma commented Feb 17, 2023

@code-asher good catch! We probably need to keep track of the deleted files 🤔.... @kylecarbs I think this feature is more tricky than it looks like. What do you think about we hide the add, delete and rename features for now? So we could safely release it until we have this figured out. Some complexity that I see is a user trying to add or rename a file type that is not supported.

@BrunoQuaresma
Copy link
Collaborator Author

Hm... thinking better, maybe it is not that much work. I will try to fix it.

@BrunoQuaresma
Copy link
Collaborator Author

@code-asher could you please try it again?

Copy link
Member

@kylecarbs kylecarbs left a comment

Choose a reason for hiding this comment

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

Didn't try this out, but feel free to merge whenever you think it's good!

Copy link
Member

@code-asher code-asher left a comment

Choose a reason for hiding this comment

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

Rename and delete are working much better now!

There are some quirks:

  • Creating a file without a certain extension (for example test or test.h) always creates a folder. Ideally I think there would be separate new folder/file buttons.
  • Renaming a file without a certain extension creates a folder. You can then do weird things like add files underneath this "folder" which is actually a file. A file should always be a file even if renamed regardless of the extension.
  • You can add a file under a file (main.tf/test.md) which seems to cause the file to get ordered like a folder but the new file will not actually appear. This should probably be disallowed.
  • Renaming a folder that has children causes a crash.
  • Renaming a folder that has no children with a certain extension (like test.md) causes it to become a file. A folder should always be a folder even if renamed regardless of the extension.
  • Sorting seems to be a bit random. If I add test it shows at the bottom then if I add test/test it shows up at the top.

@BrunoQuaresma BrunoQuaresma force-pushed the bq/refactor-rename-feature branch from 43519f6 to ab9b8da Compare March 2, 2023 14:51
@BrunoQuaresma
Copy link
Collaborator Author

I tried to solve all the Asher points but I had to disable folder creation or renaming for now. I think what we have right now is good enough but stills buggy for certain use cases. I think this is good for now and we can get back into it later and do a major refactoring and add different validation and flow for folders and files.

@BrunoQuaresma BrunoQuaresma enabled auto-merge (squash) March 2, 2023 15:03
@code-asher
Copy link
Member

Tested it out and it looks solid, the only thing I noticed is that I can still create/rename folders by leaving out the dot (for example maintf or md).

@BrunoQuaresma BrunoQuaresma merged commit a3201bd into main Mar 6, 2023
@BrunoQuaresma BrunoQuaresma deleted the bq/refactor-rename-feature branch March 6, 2023 13:53
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.

3 participants