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

feat: add possibility to write the filename per editor #52

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

dimaslz
Copy link
Contributor

@dimaslz dimaslz commented Nov 23, 2021

Hello @stevebauman here the PR about my suggestion #29

Take a look at how it looks:

With one file:
Screenshot 2022-01-01 at 14 01 43

With more than one file
Screenshot 2022-01-01 at 14 00 48
Screenshot 2022-01-01 at 14 00 39

Let me know if I can improve something in code or UI.

UPDATE: update with new UI and rebase with master

@netlify
Copy link

netlify bot commented Nov 23, 2021

✔️ 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

@stevebauman
Copy link
Owner

This is so cool @dimaslz! Nice work! 🎉

I'll review this early tomorrow, but I love this idea and the look of it 👍

@stevebauman
Copy link
Owner

Hmm -- when I add a new editor and start typing into the file name field, nothing appears on the preview window until I refresh?

@dimaslz
Copy link
Contributor Author

dimaslz commented Nov 24, 2021

Hello, @stevebauman I rebased and updated the PR, I think now is fixed. I missed the initial value.

@stevebauman
Copy link
Owner

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!

@dimaslz
Copy link
Contributor Author

dimaslz commented Nov 26, 2021

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.

@dimaslz
Copy link
Contributor Author

dimaslz commented Jan 1, 2022

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"
Copy link
Owner

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"
/>

Copy link
Contributor Author

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?

Copy link
Contributor Author

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

<div
v-if="filenames.length > 1"
class="text-sm mb-2 text-gray-400 w-full text-right"
>{{ filenames[index] }}</div>
Copy link
Owner

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! ❤️

@bdsoha
Copy link

bdsoha commented Dec 16, 2022

@dimaslz @stevebauman Any updates on this feature?

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.

None yet

3 participants