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.

Sign up for GitHub

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

Jump to bottom

Expose RTLTextPluginStatus #8864

Merged
merged 9 commits into from
Oct 15, 2019
Merged

Expose RTLTextPluginStatus #8864

merged 9 commits into from
Oct 15, 2019

Conversation

ryanhamley
Copy link
Contributor

@ryanhamley ryanhamley commented Oct 11, 2019 •
edited
Loading

Launch Checklist

fixes #7869

  • briefly describe the changes in this PR
    • this plugin exposes the RTLTextPluginStatus (which can be unavailable, loading, loaded or error) through a mapboxgl.getRTLTextStatusPlugin method
  • write tests for all new functionality
  • document any changes to public APIs
  • manually test the debug page

CHANGELOG
Expose getRTLTextPluginStatus method

@ryanhamley ryanhamley mentioned this pull request Oct 11, 2019
Closed
3 tasks
asheemmamoowala
asheemmamoowala suggested changes Oct 12, 2019
View reviewed changes
Copy link
Contributor

@asheemmamoowala asheemmamoowala left a comment

Choose a reason for hiding this comment

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

The change to add and expose pluginStatus looks good. pluginRequested can probably removed too, and replaced with pluginStatus.

@asheemmamoowala asheemmamoowala added this to the release-sangria milestone Oct 14, 2019
@ryanhamley
Copy link
Contributor Author

@asheemmamoowala I removed pluginRequested

arindam1993
arindam1993 suggested changes Oct 14, 2019
View reviewed changes
Copy link
Contributor

@arindam1993 arindam1993 left a comment

Choose a reason for hiding this comment

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

plugin.isLoaded() should probably be moved to querying the enum as well.

src/source/rtl_text_plugin.js Outdated Show resolved Hide resolved
arindam1993
arindam1993 approved these changes Oct 15, 2019
View reviewed changes
Copy link
Contributor

@arindam1993 arindam1993 left a comment

Choose a reason for hiding this comment

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

LGTM

@arindam1993 arindam1993 mentioned this pull request Oct 15, 2019
Merged
8 tasks
@ryanhamley ryanhamley merged commit 036ddc4 into master Oct 15, 2019
@ryanhamley ryanhamley deleted the rtl-status-update branch October 15, 2019 05:11
@snyk-bot snyk-bot mentioned this pull request Dec 5, 2019
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@asheemmamoowala asheemmamoowala asheemmamoowala approved these changes

@arindam1993 arindam1993 arindam1993 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
release-sangria
Development

Successfully merging this pull request may close these issues.

Expose a "clearRTLTextPlugin" or a give a way to check if the rtl plugin is requested
3 participants
@ryanhamley @arindam1993 @asheemmamoowala