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

Enable custom font entry when changing font through the command line #1003

Merged
merged 1 commit into from
Feb 27, 2021

Conversation

leduyquang753
Copy link
Contributor

This allows the user to type a custom font name into the command input box then press Enter to set a custom font in the Change font family command.

This also fixes an issue where the settings page does not reflect the current settings when directly accessing /settings.

@Miodec
Copy link
Member

Miodec commented Feb 20, 2021

This doesn't work with single list command line. Plus why go through all the issues of adding a defaultBehavior() and changing half the command line script when you could just use input: true like the pace caret fields?

{
  id: "setPaceCaretCustom",
  display: "custom...",
  input: true,
  exec: (input) => {
    setPaceCaretCustomSpeed(input);
    setPaceCaret("custom");
  },
},

Am I missing something?

@leduyquang753
Copy link
Contributor Author

Well to be honest I didn't notice that existing custom implementation, but anyway I feel it's better to directly type in the custom name instead of having to pick custom beforehand. If you still feel like this is unnecessary then I will switch.

@Miodec
Copy link
Member

Miodec commented Feb 21, 2021

I think I would prefer the command line to be consistent in its functionality, so ideally you should use the already present input behavior.

Plus as I mentioned your implementation does not support single list command line.

Also fixed an issue where the settings page does not reflect the current settings when directly accessing /settings.
@Miodec
Copy link
Member

Miodec commented Feb 24, 2021

Could you explain to me the config load promise change? You seem to know more about async stuff than me

@leduyquang753
Copy link
Contributor Author

There are two points where I made promises:

  1. configLoadPromise is waited for by fillSettingsPage. Because immediately using JQuery to set active option buttons after creating those option buttons seems to not work (properly), I add the active status directly into the buttons when filling them into the settings page, which requires the current config to be loaded.
  2. settingsFillPromise is waited for by updateSettingsPage so that by the time the function is executed, the settings page is sure to have the necessary buttons to be worked on and set active according to the current config.

@Miodec Miodec merged commit d3d1732 into monkeytypegame:master Feb 27, 2021
@leduyquang753 leduyquang753 deleted the cmd-custom-font branch March 2, 2021 11:20
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

2 participants