-
-
Notifications
You must be signed in to change notification settings - Fork 70
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
feat: add possibility to write the filename per editor #52
base: master
Are you sure you want to change the base?
Conversation
✔️ Deploy Preview for festive-hermann-8f687a ready! 🔨 Explore the source changes: 2c22446 🔍 Inspect the deploy log: https://app.netlify.com/sites/festive-hermann-8f687a/deploys/61e9d4d103cded0007de995e 😎 Browse the preview: https://deploy-preview-52--festive-hermann-8f687a.netlify.app |
This is so cool @dimaslz! Nice work! 🎉 I'll review this early tomorrow, but I love this idea and the look of it 👍 |
Hmm -- when I add a new editor and start typing into the file name field, nothing appears on the preview window until I refresh? |
5585684
to
4ba142a
Compare
Hello, @stevebauman I rebased and updated the PR, I think now is fixed. I missed the initial value. |
4ba142a
to
33cf92c
Compare
Thanks @dimaslz! Could we have these inputs only show when we have 2 or more editors? I don’t think we should have them displayed when there’s only one editor (we can use the title field for that). Let me know your thoughts! |
Mmmm... I think make sense. I will take a look. |
Hello @stevebauman, sorry for my delay. Take a look ;) |
@@ -25,6 +25,8 @@ | |||
:size="sizes[0]" | |||
:tab-size="editor.tabSize" | |||
:language="editor.language" | |||
:filename="editor.filename" | |||
:allow-filename="editors.length > 1" |
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.
Instead of a separate prop for allow-filename
, can we use a ternary condition to simply pass in null
in the filename
prop? For example:
<Editor
:filename="editors.length > 1 ? editor.filename : null"
/>
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.
Ok, I remember why. If I do not use allow-filename
, I don't know when I can allow writing the filename or not. For example, inside Editor.vue
in the v-if="allowFilename"
, we need to know how much Editors are using to allow set the filename or not. With this :filename="editors.length > 1 ? editor.filename : null"
, the filename always is allowed to set but, when just exists one Editor, the filename field is visible, but not visible in the render. Do you know what I mean?
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.
Maybe I can do something like canSetFilename
components/Window.vue
Outdated
<div | ||
v-if="filenames.length > 1" | ||
class="text-sm mb-2 text-gray-400 w-full text-right" | ||
>{{ filenames[index] }}</div> |
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.
Can we add an additional clause here to the if
statement so the <div>
isn't rendered if the filename is empty? Ex:
<div
v-if="filenames.length > 1 && filenames[index]"
>
Once these two changes are made I'll merge this in 👍
Thanks for your time and work! ❤️
@dimaslz @stevebauman Any updates on this feature? |
Hello @stevebauman here the PR about my suggestion #29
Take a look at how it looks:
With one file:
With more than one file
Let me know if I can improve something in code or UI.
UPDATE: update with new UI and rebase with
master