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

Language Improvements #13

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Language Improvements #13

wants to merge 4 commits into from

Conversation

calumk
Copy link
Owner

@calumk calumk commented Mar 9, 2025

Includes Improvements from @keertyverma and @calumk

Copy link

@keertyverma keertyverma left a comment

Choose a reason for hiding this comment

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

Added few comments. Let me know your thoughts.
Thank you.

Comment on lines +353 to +360
if(!this._isValidLanguage(language)) {
// If invalid language, fallback to plain text
this.data.language = 'plain';
this.data.editorInstance.updateLanguage('none');
this._element.querySelector('.editorjs-codeCup_LangDisplay').innerHTML = 'none';
this._handleErrorMessage(language); // Show error message
return false;
}

Choose a reason for hiding this comment

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

  • While this approach prevents invalid syntax highlighting, it may negatively impact the user experience by removing their last valid selection.

    Would it be better to retain the previously valid language and syntax highlighter instead? This would help ensure users don’t lose their last selection due to an unintended input error.

if(!this._isValidLanguage(language)) {
    this._handleErrorMessage(language); // Show an error message for the invalid language
    return false;
  }
  • Additionally, if we still want to reset the language to plain text, the language display text should also reflect Plain Text rather than showing none for consistency.

Comment on lines +55 to +77
static getDefaultLanguages() {
return {
bash: "Bash",
c: "C",
cpp: "C++",
csharp: "C#",
css: "CSS",
go: "Go",
html: "HTML",
java: "Java",
javascript: "JavaScript",
json: "JSON",
kotlin: "Kotlin",
none: "Plain Text",
php: "PHP",
python: "Python",
ruby: "Ruby",
rust: "Rust",
sql: "SQL",
swift: "Swift",
typescript: "TypeScript",
};
}

Choose a reason for hiding this comment

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

We can remove this now.

@@ -25,7 +25,7 @@
padding: 5px;
padding-left: 10px;
padding-right: 10px;
right: 0;
right: 0px;

Choose a reason for hiding this comment

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

If right: 0px is used, the language display name may not be shown properly when a scrollbar is present.
In my PR, I have adjusted this to right: 12px to ensure proper visibility.

image

.editorjs-codeCup_languageDropdown {
/* top:10px; left:100px; */
position: fixed;

Choose a reason for hiding this comment

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

With position:fixed the dropdown is cropped off in mobile devices. Would you be able to retain my changes?

	position: absolute;
	top: 100%;
	left: 0;
image

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.

2 participants