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

Better Configuration UX #342

Merged
merged 11 commits into from
Aug 26, 2021
Merged

Conversation

Christian7573
Copy link
Contributor

@Christian7573 Christian7573 commented Jun 5, 2021

I plan on embedding wetty in a project of mine and looked into changing the color theme. While the previous editor allowed changing the color theme, it was very clunky to use, needing to ready XTerm documentation to find property names and then painfully edit a tiny JSON window on my phone. I've replaced it with a simple field-value editor and copied over all of the options listed in XTerm's documentation. It looks like you have this project set up in a very specific way, so I've done my best to keep it isolated in the assets folder using vanilla HTML, CSS, and JS.

image

  • Replaced the JSON editor with a field-value GUI.
  • Modified the loading, saving, and application of XTerm configuration.
  • Old options strings will be converted to { xterm: old_options } upon using the new version.
  • Added the option to disable the XTerm Fit plugin if people choose to manually specify rows and columns.
  • Changed sizing of configuration editor to better work with smaller and mobile screens.
  • Bumped version of @typescript/eslint-plugin; NPM could not resolve the version previously listed in package.json.
  • Added package-lock.json and yarn.lock to the .gitignore.
  • Expose the Term object as window.wetty_term; Not related to this PR, but I'll be using it to access the terminal externally for my project.

@Christian7573 Christian7573 marked this pull request as ready for review June 5, 2021 02:38
@butlerx butlerx force-pushed the main branch 3 times, most recently from 8976055 to c897f2f Compare August 24, 2021 22:38
@Christian7573
Copy link
Contributor Author

Christian7573 commented Aug 25, 2021

Interesting to note: typescript/eslint-plugin 2.5.0 is no longer available from npm and only available through yarn.
Let me know if you'd like me to squash commits.

Copy link
Owner

@butlerx butlerx left a comment

Choose a reason for hiding this comment

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

I'm curious to know the reasoning behind using an iframe rather than adding to the main page?

@@ -1,7 +1,7 @@
import _ from 'lodash';

export function loadOptions(): object {
const defaultOptions = { fontSize: 14 };
export function loadOptions(): any {
Copy link
Owner

Choose a reason for hiding this comment

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

Why switch from object to any?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I believe TS didn't like me indexing/using properties from type object, however changing it back to object now does not bring that error up. Will revert.

@Christian7573
Copy link
Contributor Author

I'm curious to know the reasoning behind using an iframe rather than adding to the main page?

To have a hard boundary for CSS and JS since it's neither using SCSS nor bundled TS.

@butlerx butlerx merged commit 780b931 into butlerx:main Aug 26, 2021
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