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

Adding support for font customization #476

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

Conversation

daonb
Copy link
Contributor

@daonb daonb commented Jun 17, 2024

Summary by CodeRabbit

  • New Features
    • Added configurable font settings under [theme] in configuration files.
  • Bug Fixes
    • Improved handling for unverified hosts and multi-client layout synchronization.
    • Enhanced error handling during terminal resizing and connection processes.
  • Refactor
    • Reworked terminal emulator font loading to use FontFaceObserver.
    • Consolidated various layout and screen scaling functionalities.
  • Style
    • Set default fonts to Fira Sans and FiraCode in key UI components for consistency.

Copy link

coderabbitai bot commented Jun 17, 2024

Walkthrough

The latest updates introduce configurable themes, enhanced font handling, and layout synchronization across multiple components. The changes ensure consistent use of the Fira Sans font, handle unverified hosts, refine keyboard input handling in tests, and streamline the connection logic. Dependencies have been updated, screen scaling optimized, and state management refined. These modifications improve the usability, aesthetics, and robustness of the application.

Changes

File Summary
CHANGELOG.md Added font configuration in dotfile, resolved unverified hosts, synced multi-client layout, set Fira Sans font for rename/search.
aatp/peerbook_webrtc/admin.spec.ts Revised keyboard input logic for tests, added loop to wait for specific condition before proceeding.
css/terminal7.css Added font-family: 'FiraSans'; to button.rename-close selector.
package.json Removed @liveconfig/xterm-webfont, added fontfaceobserver dependency.
src/commands.ts Restructured connectCMD function for platform-specific gate handling and user verification.
src/gate.ts Adjusted Gate class methods and properties for layout, state, and screen scaling improvements.
src/map.ts Refactored to use FontFaceObserver for font loading, simplified terminal opening logic.
src/pane.ts Updated font handling, removed resizeObserver, added openEmulator and scaleCanvas functions.
src/shell.ts Added this.printPrompt() call in onFirstConnectionDisconnect method.
src/terminal7.ts Enhanced default configuration, updated control flow for gate resizing and error handling, improved theme settings.

Sequence Diagram(s)

New Font Loading and Terminal Opening Flow

sequenceDiagram
    participant User
    participant FontFaceObserver
    participant Terminal
    participant UI

    User->>UI: Initialize Terminal
    UI->>FontFaceObserver: Load 'FiraCode' Font
    FontFaceObserver-->>UI: Font Loaded
    UI->>Terminal: Open Terminal
    Terminal-->>UI: Terminal Ready
    UI-->>User: Terminal Initialized
Loading

Gate Connection Flow

sequenceDiagram
    participant User
    participant Shell
    participant Gate
    participant PlatformSpecificHandler

    User->>Shell: Connect to Gate
    Shell->>PlatformSpecificHandler: Check Gate Address and Username
    PlatformSpecificHandler-->>Shell: Address and Username
    Shell->>Gate: Verify Gate
    Gate-->>Shell: Verification Complete
    Shell-->>User: Connection Established
Loading

Poem

In lines of code, a rabbit writes,
New fonts to set, scaling sights.
Connection strong, themes anew,
Terminals open, work to do.
With careful hops, bugs take flight,
Oh, in the code, pure delight!
🐇✨


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

Share
Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai generate interesting stats about this repository and render them as a table.
    • @coderabbitai show all the console.log statements in this repository.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (invoked as PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Jun 17, 2024

Deploy Preview for dazzling-kringle-090742 ready!

Name Link
🔨 Latest commit 36e743d
🔍 Latest deploy log https://app.netlify.com/sites/dazzling-kringle-090742/deploys/66705c1fe0f28e0008a4ee3e
😎 Deploy Preview https://deploy-preview-476--dazzling-kringle-090742.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Jun 17, 2024

Deploy Preview for terminal7 ready!

Name Link
🔨 Latest commit 36e743d
🔍 Latest deploy log https://app.netlify.com/sites/terminal7/deploys/66705c1f8a91f300088ee44d
😎 Deploy Preview https://deploy-preview-476--terminal7.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 9

Outside diff range and nitpick comments (13)
css/terminal7.css (1)

Line range hint 105-106: Remove unexpected character to fix the parse error.

-} 
+}

This character may cause issues in some browsers or tools that do not recognize it as valid CSS syntax.

src/gate.ts (4)

Line range hint 238-239: Wrap the declaration of session in a block to restrict its scope to the switch clause.

case "set_payload":
+ {
    let session: WebRTCSession = this.session
    this.setLayout(msg.args["payload"], true)
+ }
    break

Line range hint 242-245: The else clause can be omitted for clarity and to reduce unnecessary nesting.

- else
-     Clipboard.write({image: msg.args["data"]})
- session.sendCTRLMsg(new ControlMessage("ack", {ref: msg.message_id}))
+ Clipboard.write({image: msg.args["data"]})
+ session.sendCTRLMsg(new ControlMessage("ack", {ref: msg.message_id}))

Line range hint 510-511: Change to an optional chain to simplify the code and improve safety.

- if (win && win.activeP)
+ if (win?.activeP)

368-368: The variable 'fromPresenter' is assigned but never used. Consider removing it if it's not needed.

Tools
GitHub Check: test

[warning] 368-368:
'fromPresenter' is assigned a value but never used

src/commands.ts (3)

Line range hint 339-349: Replace isNaN with Number.isNaN to avoid type coercion issues.

-    if (isNaN(sshPort) || sshPort < 1 || sshPort > 65535)
+    if (Number.isNaN(sshPort) || sshPort < 1 || sshPort > 65535)
Tools
Biome

[error] 195-195: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


Line range hint 687-690: The else clauses in the resetCMD function can be omitted for better readability and to avoid unnecessary nesting.

-    } else {
-        return
-    }

Also applies to: 951-961, 956-961, 963-963

Tools
Biome

[error] 195-195: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


Line range hint 1016-1044: Variables declared in a switch statement are accessible to all cases, which might lead to bugs. Encapsulate declarations within blocks to limit their scope.

case 'Edit':
    {
+   let enabled, res;
    enabled = await shell.runForm(fFields, "choice", `\x1B[4m${gate.name}\x1B[0m edit`)
    if (!enabled) {
        await gate.t7.clear()
        return
    }
    fFields = fFields.filter((_, i) => enabled[i])
    res = await shell.runForm(fFields, "text")
    if (isPB && enabled[0])
        await terminal7.pb.adminCommand(
            new ControlMessage("rename", { target: gate.fp, name: res[0]}))
    gateAttrs.filter((_, i) => enabled[i])
             .forEach((k, i) =>  {
                 let v = res[i]
                 if (k == "sshPort")
                     v = parseInt(v)
                 if (k == "onlySSH")
                     v = v == "y"
                 gate[k] = v
            })
    gate.t7.storeGates()
    gate.updateNameE()
    break
    }
Tools
Biome

[error] 195-195: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.

src/terminal7.ts (5)

Line range hint 203-204: Consider using optional chaining to simplify these expressions.

This can reduce complexity and increase the robustness of your code.

- if (this.activeG && this.activeG.activeW && this.activeG.activeW.activeP)
+ if (this.activeG?.activeW?.activeP)

Also applies to: 276-277, 279-280, 485-486, 569-571, 608-609, 647-647


Line range hint 481-494: The else clause after return statements in previous branches can be omitted for cleaner code.

This simplifies the control flow and improves readability.

- else {
-    // code
- }

Line range hint 669-672: Avoid using assignments within expressions as it can lead to confusing code.

Consider separating the assignment from the expression to clarify the intent.

- if ((this.keys = getKeys())) {
+ this.keys = getKeys();
+ if (this.keys) {

Also applies to: 718-719, 935-936, 1018-1019


Line range hint 789-789: Using void in union types can be confusing. Consider using undefined instead.

This improves type clarity and aligns with TypeScript best practices.

- Promise<RTCCertificate | void>
+ Promise<RTCCertificate | undefined>

Line range hint 346-346: Replace isNaN with Number.isNaN for safer type checking.

isNaN attempts type coercion which can lead to unexpected results.

- if (isNaN(runs))
+ if (Number.isNaN(runs))
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 9ff86c1 and 36e743d.

Files ignored due to path filters (1)
  • yarn.lock is excluded by !**/yarn.lock, !**/*.lock
Files selected for processing (10)
  • CHANGELOG.md (1 hunks)
  • aatp/peerbook_webrtc/admin.spec.ts (1 hunks)
  • css/terminal7.css (1 hunks)
  • package.json (2 hunks)
  • src/commands.ts (1 hunks)
  • src/gate.ts (12 hunks)
  • src/map.ts (5 hunks)
  • src/pane.ts (15 hunks)
  • src/shell.ts (1 hunks)
  • src/terminal7.ts (5 hunks)
Additional context used
Biome
src/map.ts

[error] 263-263: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 288-288: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 273-273: Use the '**' operator instead of 'Math.pow'. (lint/style/useExponentiationOperator)

Unsafe fix: Use the '**' operator instead of 'Math.pow'.

css/terminal7.css

[error] 105-106: unexpected character (parse)

src/gate.ts

[error] 238-239: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.


[error] 242-245: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 387-388: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 510-511: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 373-373: Use Array.isArray() instead of instanceof Array. (lint/suspicious/useIsArray)

instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.

src/commands.ts

[error] 195-195: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 687-690: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 951-961: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 956-961: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)


[error] 963-963: This code is unreachable (lint/correctness/noUnreachable)

... because either this statement will break the flow of the code, ...

... this statement will return from the function, ...

... this statement will break the flow of the code, ...

... this statement will return from the function, ...

... this statement will return from the function, ...

... or this statement will return from the function beforehand


[error] 975-975: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 1016-1017: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.


[error] 1023-1028: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.


[error] 1028-1029: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.


[error] 1033-1034: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.


[error] 1036-1044: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.


[error] 1053-1053: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause. (lint/correctness/noSwitchDeclarations)

The declaration is defined in this switch clause:

Unsafe fix: Wrap the declaration in a block.


[error] 1115-1115: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 339-339: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.


[error] 349-349: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.


[error] 511-511: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

src/terminal7.ts

[error] 203-204: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 276-277: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 279-280: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 481-494: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 485-486: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 569-571: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 608-609: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 647-647: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.


[error] 669-672: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 718-719: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 789-789: void is confusing inside a union type. (lint/suspicious/noConfusingVoidType)

Unsafe fix: Use undefined instead.


[error] 935-936: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 1018-1019: The assignment should not be in an expression. (lint/suspicious/noAssignInExpressions)

The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.


[error] 346-346: isNaN is unsafe. It attempts a type coercion. Use Number.isNaN instead. (lint/suspicious/noGlobalIsNan)

See the MDN documentation for more details.
Unsafe fix: Use Number.isNaN instead.

src/pane.ts

[error] 1242-1245: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

Unsafe fix: Omit the else clause.


[error] 1244-1245: This else clause can be omitted because previous branches break early. (lint/style/noUselessElse)

LanguageTool
CHANGELOG.md

[uncategorized] ~20-~20: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...ont in rename and search is set to Fira Sans like the rest of the app ## [1.10.1] -...


[grammar] ~28-~28: This expression is usually spelled with a hyphen. (NON_ANTI_JJ)
Context: ...out - Improving touch events accuracy - Non active gates are not reconnected - Camera is n...


[uncategorized] ~36-~36: Consider adding a hyphen. (TOP_LEFT_CORNER)
Context: ...w only latency and are displayed in the top right corner ### Added - support for webexe...


[grammar] ~49-~49: The word ‘install’ is not a noun. (A_INSTALL)
Context: ...stuck TWR when chossing not to copy the install command - latency display is on the gat...


[grammar] ~65-~65: Possible subject-verb agreement error. (SINGULAR_NOUN_ADV_AGREEMENT)
Context: ...t ### Fixed - PWA first time user now get an offer to install - Reconnect failure...


[grammar] ~78-~78: Two determiners in a row. Choose either “an” or “a”. (DT_DT)
Context: ...ustom SSH port - A troubleshooting page an a dev site - https://dev.terminal7.dev #...


[typographical] ~104-~104: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...peerbook spinner and indicator ## 1.7.1 - 2023/8/27 ### Fixed - Removing WebRTC ...


[typographical] ~110-~110: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...TC stats when closing the gate ## 1.7.0 - 2023/8/24 ### Added - Images supporte...


[uncategorized] ~115-~115: If this is a compound adjective that modifies the following noun, use a hyphen. (EN_COMPOUND_ADJECTIVE_INTERNAL)
Context: ...rough sixel - requires imgcat - WebRTC real time stats ### Fixed - gates command out...


[typographical] ~123-~123: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...n for the first time in a pane ## 1.6.0 - 2023/8/17 ### Added - PeerBook gates ...


[typographical] ~136-~136: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...e message is shown when needed ## 1.5.4 - 2023/8/10 ### Fixed - Stoping watchdog...


[typographical] ~142-~142: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...g watchdog on resore purchases ## 1.5.3 - 2023/8/10 ### Added - Support for j & ...


[typographical] ~150-~150: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...store purchases - Install menu ## 1.5.2 - 2023/8/4 ### Added - Cmd-V support in ...


[typographical] ~163-~163: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ... The close command - Messaging ## 1.5.1 - 2023/8/1 ### Fixed - TWR prompt is no ...


[typographical] ~174-~174: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...inks to Privacy policy and ToS ## 1.5.0 - 2023/7/21 ### Fixed - added a reset fo...


[typographical] ~184-~184: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...l7 is now T7 on narrow screens ## 1.4.2 - 2023/1/31 ### Fixed - TWR is not scale...


[typographical] ~192-~192: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...shown only on first activation ## 1.4.1 - 2023/1/26 ### Added - gate name compl...


[typographical] ~203-~203: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ... peerbook bugs - flaky hourglass ## 1.4 - 2023/1/14 ### Added - hourglass - Rest...


[typographical] ~212-~212: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ... a websocket signaling server. ## 1.3.2 - 2023/1/10 ### Fixed - peerbook connec...


[typographical] ~218-~218: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...# Fixed - peerbook connection ## 1.3.1 - 2022/12/29 ### Added - Biometric veri...


[typographical] ~230-~230: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...s - SSH only flag is now saved ## 1.3.0 - 2022/12/16 ### Added - ED25519 keys wi...


[typographical] ~247-~247: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...end - Improving error messages ## 1.2.1 - 2022/11/10 ### Fixed - webexec install...


[typographical] ~255-~255: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...asked for only in the first time ## 1.2 - 2022/11/6 ### Added - Video button, re...


[grammar] ~263-~263: The auxiliary verb ‘do’ requires the base form of the verb. (DID_BASEFORM)
Context: ... ### Fixed - Search not found does not opens TWR - To avoid accidential splits, vert...


[uncategorized] ~264-~264: “to” seems less likely than “too” (as well, very, ‘too much’). (AI_HYDRA_LEO_CP_TO_TOO)
Context: ...as by default a very fast minimum speed to. - Copy mode help is now displayed in t...


[typographical] ~268-~268: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...s updated with the latest keys ## 1.1.1 - 2022/11/3 ### Fixed - TWR staying with...


[uncategorized] ~272-~272: It seems likely that a singular genitive (’s) apostrophe is missing. (AI_HYDRA_LEO_APOSTROPHE_S_XS)
Context: ... TWR staying with tiny font even though its blown up ## 1.1 - 2022/11/1 ### Added...


[typographical] ~274-~274: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ...ny font even though its blown up ## 1.1 - 2022/11/1 ### Added - WebRTC signaling...


[uncategorized] ~278-~278: Did you mean: “By default,”? (BY_DEFAULT_COMMA)
Context: ...### Added - WebRTC signaling over SSH. By default connection start with ssh and then try ...


[typographical] ~287-~287: If you want to indicate numerical ranges or time ranges, consider using an en dash. (DASH_RULE)
Context: ... the gate instead of deleting it ## 1.0 - 2022/10/2 ### Fixed - A nav bar only ...


[grammar] ~317-~317: Modal verbs like ‘can’ or ‘will’ require the following verb to be in its base form. (MD_BASEFORM)
Context: ...help page is displayed when it should - Handling of hosts that refuse the fingerprint - ...


[style] ~332-~332: Consider replacing this phrase with the adverb “nicely” to avoid wordiness. (IN_A_X_MANNER)
Context: ...ents - Interrupting the welcome message in a nice way ## [1.0-rc.2] - 2022/9/11 Lost in a r...


[uncategorized] ~348-~348: The adjective “on-screen” is spelled with a hyphen. (ON_SITE_HYPHEN)
Context: ...# Fixed - Resize the terminal when the on screen keyboard is displayed/hidden ## [0.21....


[misspelling] ~433-~433: This word is normally spelled as one. (EN_COMPOUNDS_SUPER_FAST)
Context: ... 2022/4/10 ### Changed - Rendering is super fast thanks to WebGL hardware acceleration -...


[style] ~555-~555: This phrase is redundant. Consider writing “exiting”. (EXIT_FROM)
Context: ... ## [0.16.1] - 2021/8/15 ### Fixed - exiting from zoom ## [0.16.0] - 2021/8/11 ### Adde...


[style] ~569-~569: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym. (ADVERB_REPETITION_PREMIUM)
Context: ...nnect - properly handle missing shell - properly restoring a zoomed session - when contr...


[grammar] ~576-~576: Possibly, an article or a pronoun is missing between ‘in’ and ‘home’. For the meaning of “in one’s own house”, the usual preposition is ‘at’. Did you mean “at home”? (IN_HOME)
Context: ...21/5/21 ### Added - secondary nav bar in home with + and reset ### Fixed - connecti...


[grammar] ~652-~652: Possibly, an article or a pronoun is missing between ‘in’ and ‘home’. For the meaning of “in one’s own house”, the usual preposition is ‘at’. Did you mean “at home”? (IN_HOME)
Context: ...ges moved to the side - same help shown in home and inside a gate ### Fixed - communi...


[uncategorized] ~774-~774: Use a comma before ‘but’ if it connects two independent clauses (unless they are closely connected and short). (COMMA_COMPOUND_SENTENCE_2)
Context: ...keys ### Fixed - Copy mode had limited but complete functionality - Like always, r...


[uncategorized] ~775-~775: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...ality - Like always, reconnect is a bit better should be working fine on single window...


[uncategorized] ~844-~844: “to” seems less likely than “too” (as well, very, ‘too much’). (AI_HYDRA_LEO_CP_TO_TOO)
Context: ...07/20 ### Fixed - Terminals are never to high and the last line is never clipped...

Markdownlint
CHANGELOG.md

3-3: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


96-96: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


99-99: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


112-112: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


114-114: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


125-125: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


154-154: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


158-158: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


167-167: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


194-194: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


212-212: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


220-220: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


278-278: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


289-289: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


297-297: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


409-409: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


411-411: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


418-418: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


437-437: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


439-439: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


445-445: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


452-452: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


488-488: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


553-553: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


564-564: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


587-587: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


613-613: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


619-619: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


627-627: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


649-649: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


661-661: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


669-669: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


685-685: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


693-693: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


699-699: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


744-744: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


752-752: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


780-780: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


822-822: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


861-861: Expected: 0 or 2; Actual: 1 (MD009, no-trailing-spaces)
Trailing spaces


421-421: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines


519-519: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines


666-666: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines


777-777: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines


826-826: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines


839-839: Expected: 1; Actual: 2 (MD012, no-multiple-blanks)
Multiple consecutive blank lines


511-511: null (MD019, no-multiple-space-atx)
Multiple spaces after hash on atx style heading


385-385: Expected: 1; Actual: 0; Below (MD022, blanks-around-headings)
Headings should be surrounded by blank lines


386-386: null (MD032, blanks-around-lists)
Lists should be surrounded by blank lines


78-78: null (MD034, no-bare-urls)
Bare URL used

GitHub Check: test
src/gate.ts

[warning] 368-368:
'fromPresenter' is assigned a value but never used

Additional comments not posted (19)
package.json (1)

50-50: Approved the addition of fontfaceobserver. This change is crucial for the new font loading mechanism in the terminal application.

css/terminal7.css (1)

370-370: Ensure the font-family property is applied consistently across the UI.

This change aligns with the PR's objective to standardize font settings across the project.

CHANGELOG.md (2)

13-14: The addition of configurable font settings is well-documented, enhancing user customization.


18-20: Ensure consistency in the application of font settings across different parts of the application.

Tools
LanguageTool

[uncategorized] ~20-~20: Possible missing comma found. (AI_HYDRA_LEO_MISSING_COMMA)
Context: ...ont in rename and search is set to Fira Sans like the rest of the app ## [1.10.1] -...

src/shell.ts (1)

529-529: Ensure that the printPrompt method is called at appropriate places to maintain the prompt's visibility.

src/gate.ts (3)

23-23: Ensure the TOOLBAR_HEIGHT constant is used consistently throughout the project.


68-68: The addition of lastState as a property in the Gate class is crucial for state management. Ensure it is properly initialized and used throughout the class.


94-94: Setting fitScreen to false by default may affect the initial display of UI elements. Verify if this aligns with the intended user experience.

src/commands.ts (1)

209-232: The added logic for gate address and username retrieval is a significant improvement. However, it's essential to handle exceptions gracefully and provide meaningful error messages to the user.

src/terminal7.ts (4)

78-86: Added default font settings for font_family and font_size under the [theme] section in the TOML configuration.

This aligns with the PR's objective to support font customization.


363-363: Use of ResizeObserver to handle resizing of gates.

This is a good use of modern web APIs to enhance responsiveness.


440-440: Enhanced error handling in pbConnect method by logging the failure.

This improves debugging and user feedback on connection issues.


1090-1100: The saveDotfile method now triggers a configuration reload and theme update across all cells when the dotfile is saved.

This ensures that any changes in the configuration are immediately reflected in the UI.

src/pane.ts (6)

9-9: Import of openEmulator added to support new font handling logic.


218-222: Good implementation of dynamic canvas scaling based on font size changes.


226-229: Efficient handling of theme changes with conditional checks to minimize unnecessary operations.


672-678: Implemented draggable functionality with clean separation of move and end event handling.


695-696: Conditional visibility of dividers based on fitScreen status is a good touch for dynamic UI adjustments.


474-474: Adjusts the height of the zoomed pane based on the platform, ensuring appropriate visibility and interaction space.

import interact from 'interactjs'
import FontFaceObserver from "fontfaceobserver"
Copy link

Choose a reason for hiding this comment

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

Approved the integration of FontFaceObserver for dynamic font loading. Consider enhancing the error handling in the catch block to log specific errors or notify the user.

-            t.open(e)
+            console.error("Error loading font:", e);
+            t.open(e);

Also applies to: 24-42

Comment on lines +93 to +96
this.fontSize = props.fontSize || terminal7.conf.theme.fontSize
} catch (e) {
this.fontSize = 14
}
Copy link

Choose a reason for hiding this comment

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

Ensure that the catch block logs the error for better debugging and maintenance.

-        } catch (e) {
+        } catch (e) {
+            console.error("Error setting fontSize:", e);
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
this.fontSize = props.fontSize || terminal7.conf.theme.fontSize
} catch (e) {
this.fontSize = 14
}
this.fontSize = props.fontSize || terminal7.conf.theme.fontSize
} catch (e) {
this.fontSize = 14
console.error("Error setting fontSize:", e);

Comment on lines +200 to +205
let ready = false
while (!ready) {
await sleep(100)
const twr = await getTWRBuffer(page)
ready = twr.match(/Name \[webexec\]:/)
}
Copy link

Choose a reason for hiding this comment

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

Approved the addition of the loop to wait for a specific condition, enhancing test robustness. Consider adding a timeout mechanism to avoid potential infinite loops.

let attempts = 0;
while (!ready && attempts < 10) {
    await sleep(100);
    const twr = await getTWRBuffer(page);
    ready = twr.match(/Name \[webexec\]:/);
    attempts++;
}
if (!ready) throw new Error("Timeout waiting for condition");

Comment on lines +723 to +745
H = this.w.e.offsetHeight,
gate = this.gate
let d = this.dividers[0]
function setVisibility(e: HTMLElement) {
if (gate.fitScreen)
e.classList.remove("hidden")
else
e.classList.add("hidden")
}

if (this.xoff > 0.001 && this.sy * H > 50) {
// refresh left divider position
d.style.left = `${this.xoff * W - 4 - 20 }px`
d.style.top = `${(this.yoff + this.sy/2)* H - 22 - 40}px`
d.classList.remove("hidden")
setVisibility(d)
} else
d.classList.add("hidden")
d = this.dividers[1]
if (this.yoff > 0.001 && this.sx * W > 50) {
// refresh top divider position
d.style.top = `${this.yoff * H - 25 - 20 }px`
d.style.left = `${(this.xoff + this.sx/2)* W - 22 - 40}px`
d.classList.remove("hidden")
setVisibility(d)
Copy link

Choose a reason for hiding this comment

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

Well-implemented dynamic adjustment of divider positions and visibility. Consider abstracting repetitive style calculations to a helper function for clarity.

+ function calculateStyle(xoff, yoff, W, H) {
+     return {
+         left: `${xoff * W - 4 - 20}px`,
+         top: `${yoff * H - 22 - 40}px`
+     };
+ }
-            d.style.left = `${this.xoff * W - 4 - 20 }px`
-            d.style.top = `${(this.yoff + this.sy/2)* H - 22 - 40}px`
+            const style = calculateStyle(this.xoff, this.sy/2, W, H);
+            d.style.left = style.left;
+            d.style.top = style.top;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
H = this.w.e.offsetHeight,
gate = this.gate
let d = this.dividers[0]
function setVisibility(e: HTMLElement) {
if (gate.fitScreen)
e.classList.remove("hidden")
else
e.classList.add("hidden")
}
if (this.xoff > 0.001 && this.sy * H > 50) {
// refresh left divider position
d.style.left = `${this.xoff * W - 4 - 20 }px`
d.style.top = `${(this.yoff + this.sy/2)* H - 22 - 40}px`
d.classList.remove("hidden")
setVisibility(d)
} else
d.classList.add("hidden")
d = this.dividers[1]
if (this.yoff > 0.001 && this.sx * W > 50) {
// refresh top divider position
d.style.top = `${this.yoff * H - 25 - 20 }px`
d.style.left = `${(this.xoff + this.sx/2)* W - 22 - 40}px`
d.classList.remove("hidden")
setVisibility(d)
H = this.w.e.offsetHeight,
gate = this.gate
let d = this.dividers[0]
function setVisibility(e: HTMLElement) {
if (gate.fitScreen)
e.classList.remove("hidden")
else
e.classList.add("hidden")
}
function calculateStyle(xoff, yoff, W, H) {
return {
left: `${xoff * W - 4 - 20}px`,
top: `${yoff * H - 22 - 40}px`
};
}
if (this.xoff > 0.001 && this.sy * H > 50) {
// refresh left divider position
const style = calculateStyle(this.xoff, this.sy/2, W, H);
d.style.left = style.left;
d.style.top = style.top;
setVisibility(d)
} else
d.classList.add("hidden")
d = this.dividers[1]
if (this.yoff > 0.001 && this.sx * W > 50) {
// refresh top divider position
d.style.top = `${this.yoff * H - 25 - 20 }px`
d.style.left = `${(this.xoff + this.sx/2)* W - 22 - 40}px`
setVisibility(d)

const winLen = this.windows.length
if(this.fitScreen)
this.fontScale = 1
const container = this.e.querySelector(".windows-container") as HTMLDivElement
if ((state == null) || !(state.windows instanceof Array) || (state.windows.length == 0)) {
Copy link

Choose a reason for hiding this comment

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

Use Array.isArray() instead of instanceof Array for more reliable type checking.

- if ((state == null) || !(state.windows instanceof Array) || (state.windows.length == 0)) {
+ if ((state == null) || !Array.isArray(state.windows) || (state.windows.length == 0)) {
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if ((state == null) || !(state.windows instanceof Array) || (state.windows.length == 0)) {
if ((state == null) || !Array.isArray(state.windows) || (state.windows.length == 0)) {
Tools
Biome

[error] 373-373: Use Array.isArray() instead of instanceof Array. (lint/suspicious/useIsArray)

instanceof Array returns false for array-like objects and arrays from other execution contexts.
Unsafe fix: Use Array.isArray() instead.

Comment on lines +387 to +388
if (this.activeW && this.activeW.activeP.zoomed)
this.activeW.activeP.unzoom()
Copy link

Choose a reason for hiding this comment

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

Change to an optional chain to simplify the code and improve safety.

- if (this.activeW && this.activeW.activeP.zoomed)
+ if (this.activeW?.activeP?.zoomed)

Committable suggestion was skipped due to low confidence.

Tools
Biome

[error] 387-388: Change to an optional chain. (lint/complexity/useOptionalChain)

Unsafe fix: Change to an optional chain.

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.

1 participant