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

Localize simulator #2368

Closed

Conversation

pavanjoshi914
Copy link
Member

@pavanjoshi914 pavanjoshi914 commented Jul 30, 2021

Localize simulator
This PR is general PR for review, code is merged in the codebase via separate PR's

Describe the changes you have made in this PR -

  1. check and localize 36 JS configuration modules - panels , prompts etc (simulator/src/)
  2. check and localize 50 element modules (simulator/src/modules/)
  3. check and localize sequential element and testbench modules (simulator/src/testbench, simulator/src/sequential)
  4. check and localize themes and hotkey binder

Rest of the strings belong to ERB template in Rails side, PR contains complete localization in JS side

Note: Please check Allow edits from maintainers. if you would like us to assist in the PR.

localize combinational analysis prompt
populate JSON with strings for combinationalAnalysis
introdue banana-i18n in combinationalAnalysis
localize tooltip
avoid duplication in embed.js and sequential/clock.js
modules
testbench
sequential
correction in duplication of keys
theme names localization is not possible currently as they are json keys rendered on frontend
this is because when theme is set it is stored into local storage now when user changes locale we will have json key in hindi but stored key will be in english varilable will hold undefined and application will break
@commit-lint
Copy link

commit-lint bot commented Jul 30, 2021

Contributors

pavanjoshi914

Commit-Lint commands

You can trigger Commit-Lint actions by commenting on this PR:

  • @Commit-Lint merge patch will merge dependabot PR on "patch" versions (X.X.Y - Y change)
  • @Commit-Lint merge minor will merge dependabot PR on "minor" versions (X.Y.Y - Y change)
  • @Commit-Lint merge major will merge dependabot PR on "major" versions (Y.Y.Y - Y change)
  • @Commit-Lint merge disable will desactivate merge dependabot PR
  • @Commit-Lint review will approve dependabot PR
  • @Commit-Lint stop review will stop approve dependabot PR

Copy link

@accesslint accesslint bot left a comment

Choose a reason for hiding this comment

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

There are accessibility issues in these changes.

);
$("#clockProperty").append(
`<div>Time: <input class='objectPropertyAttributeEmbed' min='50' type='number' style='width:48px' step='10' name='changeClockTime' value='${simulationArea.timePeriod}'></div>`
`<div>${banana.i18n('embed-clock-property-time-period')} <input class='objectPropertyAttributeEmbed' min='50' type='number' style='width:48px' step='10' name='changeClockTime' value='${simulationArea.timePeriod}'></div>`
Copy link

Choose a reason for hiding this comment

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

Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

);
$("#clockProperty").append(
`<div>Clock: <label class='switch'> <input type='checkbox' ${
`<div>${banana.i18n('clock')} <label class='switch'> <input type='checkbox' ${
Copy link

Choose a reason for hiding this comment

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

Looks like this element is missing an accessible name or label. That makes it hard for people using screen readers or voice control to use the control.

$('#moduleProperty-inner').append(`<p><span>${banana.i18n('ux-module-property-project')}:</span> <input id='projname' class='objectPropertyAttribute' type='text' autocomplete='off' name='setProjectName' value='${getProjectName() || banana.i18n('untitled')}'></p>`);
$('#moduleProperty-inner').append(`<p><span>${banana.i18n('ux-module-property-circuit')}:</span> <input id='circname' class='objectPropertyAttribute' type='text' autocomplete='off' name='changeCircuitName' value='${globalScope.name || banana.i18n('untitled')}'></p>`);
$('#moduleProperty-inner').append(`<p><span>${banana.i18n('ux-module-property-clock-time')}:</span> <input class='objectPropertyAttribute' min='50' type='number' style='width:100px' step='10' name='changeClockTime' value='${simulationArea.timePeriod}'></p>`);
$('#moduleProperty-inner').append(`<p><span>${banana.i18n('ux-module-property-clock-enable')}:</span> <label class='switch'> <input type='checkbox' ${['', 'checked'][simulationArea.clockEnabled + 0]} class='objectPropertyAttributeChecked' name='changeClockEnable' > <span class='slider'></span></label></p>`);
Copy link

Choose a reason for hiding this comment

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

Similar blocks of code found in 2 locations. Consider refactoring.

@codeclimate
Copy link

codeclimate bot commented Jul 30, 2021

Code Climate has analyzed commit 6e6f975 and detected 22 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1
Duplication 21

View more on Code Climate.

@pavanjoshi914 pavanjoshi914 marked this pull request as draft July 30, 2021 13:45
@pavanjoshi914
Copy link
Member Author

@Shivansh2407 @ShreyaPrasad1209 I would like to have your review and opinions for translations in this PR. also I would request you to test this PR locally and provide your opinions and suggestions for the same.

Thanks in advance

Copy link
Member

@satu0king satu0king left a comment

Choose a reason for hiding this comment

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

Looks good to me

@tachyons
Copy link
Member

tachyons commented Feb 3, 2023

We can move this to vue-js simulator

@tachyons tachyons closed this Feb 3, 2023
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.

3 participants