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

chore(web): drops alignInputs, hideInputs 📴 #7360

Merged
merged 5 commits into from
Oct 17, 2022

Conversation

jahorton
Copy link
Contributor

@jahorton jahorton commented Sep 27, 2022

A follow-up to #7343.

I could've included it as part of that one, but these changes were easy enough to isolate and may be worth some discussion in isolation, given the old waitForFonts method. It is my understanding that waitForFonts existed to prevent a "flash of unstyled content" effect on touch-alias elements that would otherwise result from asynchronous font loads.

@keymanapp-test-bot skip

@jahorton jahorton added this to the A16S11 milestone Sep 27, 2022
@keymanapp-test-bot keymanapp-test-bot bot added the user-test-missing User tests have not yet been defined for the PR label Sep 27, 2022
@keymanapp-test-bot
Copy link

keymanapp-test-bot bot commented Sep 27, 2022

User Test Results

Test specification and instructions

User tests are not required

@jahorton jahorton force-pushed the change/web/drop-alignInputs branch from 1c28631 to e579530 Compare September 27, 2022 04:57
@jahorton jahorton force-pushed the change/web/drop-touch-aliases branch 2 times, most recently from 5825921 to 897d3d7 Compare September 29, 2022 01:21
@jahorton jahorton force-pushed the change/web/drop-alignInputs branch from e579530 to d1887a5 Compare September 29, 2022 01:57
@jahorton jahorton marked this pull request as ready for review September 30, 2022 02:00
@keymanapp-test-bot keymanapp-test-bot bot removed the user-test-missing User tests have not yet been defined for the PR label Sep 30, 2022
Copy link
Member

@mcdurdin mcdurdin left a comment

Choose a reason for hiding this comment

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

Looking good. I think we can go a couple of steps further, removing more util code, which maketh me happy!

Comment on lines 116 to 118
// Legacy API function for touch-alias issue workarounds
// Touch-aliases have been eliminated, though.
['alignInputs'](eleList?: HTMLElement[]){}
Copy link
Member

Choose a reason for hiding this comment

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

Why are we keeping this here? Is it just because it was a published and documented API?

In this situation, we then need a PR against the documentation to mark it as deprecated, so it can be removed in a future version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jahorton
Copy link
Contributor Author

After a bit more investigation into any possible extra avenues for cleanup, I've documented #7387. It's not 100% straightforward, so it should probably be left out of this PR.

@mcdurdin mcdurdin modified the milestones: A16S12, A16S13 Oct 16, 2022
Base automatically changed from change/web/drop-touch-aliases to master October 17, 2022 03:18
@jahorton jahorton merged commit 690a282 into master Oct 17, 2022
@jahorton jahorton deleted the change/web/drop-alignInputs branch October 17, 2022 03:19
@keyman-server
Copy link
Collaborator

Changes in this pull request will be available for download in Keyman version 16.0.82-alpha

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants