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

[fv_nsilxcen]Added and replaced chars, changes to mobile keys #2309

Merged

Conversation

HopsAndHops
Copy link
Contributor

@HopsAndHops HopsAndHops commented Jul 20, 2023

  • Replaced x̌ key with combining caron on mobile and desktop keyboards on request
  • Added rules to create NFC Unicode output where applicable
  • Replaced barred l with belted l
  • Added missing characters to accommodate language variations
  • Updated documentation for mobile and desktop
  • Approved by @caforbes

- Replaced x̌ key with combining caron on mobile and desktop keyboards on request
- Added rules to create NFC Unicode output where applicable
- Replaced barred l with belted l
- Added missing characters to accomodate language variations
@keyman-server
Copy link
Collaborator

This pull request is from an external repo and will not automatically be built. The build must still be passed before it can be merged. Ask one of the team members to make a manual build of this PR.

@DavidLRowe
Copy link
Contributor

The changes to the keyboard itself seem fine, though I didn't test them exhaustively.

Some other files need attention.

In the fv_nsilxcen/ folder, there are some files which should not be in this folder.
nsilxcenU_.png
nsilxcenU_RA.png
nsilxcenU_S.png
nsilxcenU_SRA.png
Perhaps you meant to put them in the welcome/ folder and reference them in welcome.htm? At any rate they should be removed from this folder.

In the fv_nsilxcen/source/welcome folder there is an unreferenced file:
Nsilxcen.png
It should either be removed or be referenced by welcome.htm.

In the fv_nsilxcen/source/help folder, the fv_nsilxcen.php file references "Nsilxcen.png" but the file in help/ folder is "NsilxcenU_.png". The names need to match (and it's best if they match upper/lower case as well, since Linux and Mac file systems are case sensitive).

(Optional FYI) The fv_nsilxcen.php file includes <div id='osk'> which will display the unshifted and shifted desktop keyboard layers. You can add additional parameters to display other layers (RightAlt, etc).

@HopsAndHops
Copy link
Contributor Author

HopsAndHops commented Jul 20, 2023 via email

@DavidLRowe
Copy link
Contributor

No need to make a new pull request. Just make those changes in the fv_nsilxcen branch and they will become part of this pull request. (https://github.com/First-Peoples-Cultural-Council/keyboards/tree/fv_nsilxcen)

@DavidLRowe DavidLRowe changed the title Added and replaced chars, changed to mobile keys [fv_nsilxcen]Added and replaced chars, changed to mobile keys Jul 25, 2023
@DavidLRowe
Copy link
Contributor

Hi Meryl,
Those changes look good. Just a few things more to fix.

(1) In the package source (.kps file), the reference to nsilxcenU_.png points to the file in the root folder (now deleted). It needs to change to point to the file in the welcome folder. (Easiest to use Keyman Developer and remove the existing file, then click Add and navigate to the welcome folder to select the correct file.) This incorrect reference didn't show up until those extra files were deleted.

(2) You need to bring the "fv_nsilxcen" branch (which is the basis for this pull request) up-to-date before it can be merged into the repository.
image

You can see that it's "728 commits behind". The space to the right of that message is blank in this screenshot because I don't have the permissions to make such a change to your fork of the repository. If I look at my fork of the repository, which is only 3 commits behind, I see:
image
Clicking on "Sync fork" then "Update branch" should bring your fork up-to-date.

Note that the above will only update your fork on GitHub. If you have a local copy of the keyboards repository on your computer, you'll need to update that as well.

Let me know if you have any questions.

@HopsAndHops
Copy link
Contributor Author

Done! Thanks so much for all the help, I really appreciate it.

@DavidLRowe
Copy link
Contributor

I'm not seeing the change requested in (1) above.

I think you've addressed (2) the out-of-date repository.

@HopsAndHops HopsAndHops changed the title [fv_nsilxcen]Added and replaced chars, changed to mobile keys [fv_nsilxcen]Added and replaced chars, changes to mobile keys Jul 27, 2023
@DavidLRowe
Copy link
Contributor

I think we're almost there.

Unfortunately, the last change you made (to have the .php help file reference the image in the welcome folder) won't work. The .php file in the help folder supplies the help located online on the keyman site. The welcome folder provides the help that is packaged with the keyboard. It would be nice if there could be one source, but that's not how things currently stand.

So, please add the nsilxcenU_.png file back to the help folder and change the reference in the .php file to point to it (rather than to the copy in the welcome folder).

@DavidLRowe
Copy link
Contributor

The change to the help file (to restore the image) looks good.

The only thing is that the image referenced by the .php help file doesn't need to be in the package. (I know this is confusing. The welcome folder files (welcome.htm and files it references) provide the help information included with the package, so they all need to be in the package. The help folder files (the .php file and any files it references) provide the online help, so none of those files need to be in the package.)

So, the final change (unless I find something else, of course!) you need to make is to remove the reference to nsilxcenU_help.png in the list of files included in the package.

original commit would not push for some reason, attempting to push again
Copy link
Contributor

@DavidLRowe DavidLRowe left a comment

Choose a reason for hiding this comment

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

Thanks for your perseverance!

@DavidLRowe DavidLRowe merged commit fcd8fe3 into keymanapp:master Jul 28, 2023
2 checks passed
@caforbes caforbes deleted the fv_nsilxcen branch July 29, 2023 00:06
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

3 participants