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

1.5.0-beta.1 #8879

Merged
merged 6 commits into from
Oct 17, 2019
Merged

1.5.0-beta.1 #8879

merged 6 commits into from
Oct 17, 2019

Conversation

arindam1993
Copy link
Contributor

Updated CHANGELOG.md and package.version

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.

This seems to be missing an entry for #8868

CHANGELOG.md Outdated
## 🍏 Improvements
* `icon-text-fit` now properly respects `text-writing-mode`:`vertical` ([#8835](https://github.com/mapbox/mapbox-gl-js/pull/8835))
* GeolocateControl emits an `outofmaxbounds` and cancels the geolocate `flyTo` if the user is outside of `map.maxBounds` ([#8756](https://github.com/mapbox/mapbox-gl-js/pull/8756)) (h/t [MoradiDavijani](https://github.com/MoradiDavijani))
* Symbol's fade faster when zooming out quickly reducing overlap. ([#8628](https://github.com/mapbox/mapbox-gl-js/pull/8628))
Copy link
Contributor

Choose a reason for hiding this comment

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

Symbol'sSymbols

CHANGELOG.md Outdated
## 🐞 Bug Fixes

* Fix opacity interpolation for composition expressions. ([#8818](https://github.com/mapbox/mapbox-gl-js/pull/8818))
* Fix `styleimagemissing` compatibility issues with `image` expression. ([#8839](https://github.com/mapbox/mapbox-gl-js/pull/8839) and [#8775](https://github.com/mapbox/mapbox-gl-js/pull/8775))
Copy link
Contributor

Choose a reason for hiding this comment

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

This is already in the 1.4.1 patch

CHANGELOG.md Outdated
* `icon-text-fit` now properly respects `text-writing-mode`:`vertical` ([#8835](https://github.com/mapbox/mapbox-gl-js/pull/8835))
* GeolocateControl emits an `outofmaxbounds` and cancels the geolocate `flyTo` if the user is outside of `map.maxBounds` ([#8756](https://github.com/mapbox/mapbox-gl-js/pull/8756)) (h/t [MoradiDavijani](https://github.com/MoradiDavijani))
* Symbols fade faster when zooming out quickly, reducing overlap. ([#8628](https://github.com/mapbox/mapbox-gl-js/pull/8628))
* Reduce memory usage on mouseenter event in vector tile layer's with large strings. ( [#8863](https://github.com/mapbox/mapbox-gl-js/pull/8863))
Copy link
Contributor

Choose a reason for hiding this comment

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

Description is a symptom of the root problem, maybe instead: "Reduce memory usage for vector tiles that contain long strings in feature properties."

Copy link
Contributor

@chloekraw chloekraw left a comment

Choose a reason for hiding this comment

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

thanks @arindam1993! just a few comments

CHANGELOG.md Outdated
## 1.5.0-beta.1

## ✨ Features
* GeolocateControl displays a `disabled` icon if user denies geolocation permission. [#8871](https://github.com/mapbox/mapbox-gl-js/pull/8871))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: preferably entries all start with a verb that indicates what we did. so in this case, perhaps: "Add disabled icon to GeolocateControl if user denies geolocation permission."

Copy link
Contributor

Choose a reason for hiding this comment

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

Also what makes this change in the "Features" section but the other change to GeolocateControl in the "Improvements" section? I'm having a hard time distinguishing between the two sections 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I moved it into the Features section, that does make more sense.

CHANGELOG.md Outdated

## ✨ Features
* GeolocateControl displays a `disabled` icon if user denies geolocation permission. [#8871](https://github.com/mapbox/mapbox-gl-js/pull/8871))
* Add `mapboxgl.getRTLTextPluginStatus()` to query the current status of the `rtl-text-plugin` ([#8864](https://github.com/mapbox/mapbox-gl-js/pull/8864))
Copy link
Contributor

Choose a reason for hiding this comment

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

what benefit does this provide for the customer? what can I do now with the RTL text plugin that I couldn't do before?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Rephrased, and referenced the issue as well.

CHANGELOG.md Outdated
## ✨ Features
* GeolocateControl displays a `disabled` icon if user denies geolocation permission. [#8871](https://github.com/mapbox/mapbox-gl-js/pull/8871))
* Add `mapboxgl.getRTLTextPluginStatus()` to query the current status of the `rtl-text-plugin` ([#8864](https://github.com/mapbox/mapbox-gl-js/pull/8864))
* `hash` Map option can now be set as a string which sets the map hash to a custom query parameter. ([#8603](https://github.com/mapbox/mapbox-gl-js/pull/8603)) (h/t [SebCorbin](https://github.com/SebCorbin))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: remove "now", it's unnecessary

CHANGELOG.md Outdated

* Fix opacity interpolation for composition expressions. ([#8818](https://github.com/mapbox/mapbox-gl-js/pull/8818))
* Fix rotate and pitch events being fired at the same time. ([#8872](https://github.com/mapbox/mapbox-gl-js/pull/8872))
* Fix memory leaks w.r.t tile loading and map removal.([#8813](https://github.com/mapbox/mapbox-gl-js/pull/8813) and [#8850](https://github.com/mapbox/mapbox-gl-js/pull/8850))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I think it's less confusing, esp to a global audience, to avoid using abbreviations like w.r.t.

How is "Fix memory leaks in tile loading and map removal." ?

CHANGELOG.md Outdated
* GeolocateControl emits an `outofmaxbounds` and cancels the geolocate `flyTo` if the user is outside of `map.maxBounds` ([#8756](https://github.com/mapbox/mapbox-gl-js/pull/8756)) (h/t [MoradiDavijani](https://github.com/MoradiDavijani))
* Symbols fade faster when zooming out quickly, reducing overlap. ([#8628](https://github.com/mapbox/mapbox-gl-js/pull/8628))
* Reduce memory usage for vector tiles that contain long strings in feature properties. ( [#8863](https://github.com/mapbox/mapbox-gl-js/pull/8863))
* Improved icon density for `text-variable-anchor` by trying additional placements when icon gets collided out. ([#8803](https://github.com/mapbox/mapbox-gl-js/pull/8803))
Copy link
Contributor

Choose a reason for hiding this comment

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

hmm, just from reading this changelog entry, I'm a bit confused what the change was in this release because this entry seems to describe the intention of the variable label placement feature.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The change is in the icon part, we wouldn't try additional placements if the icon associated with the text got collided out, I made the description a bit clearer.

Copy link
Contributor

Choose a reason for hiding this comment

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

Got it. Reading your updated entry, I also think it's more of a bug fix, as when we release new style spec properties i.e. text-variable-anchor, the expectation is that they work with all existing properties i.e. icon-text-fit. (aside: In retrospect, maybe symbol-variable-anchor would have been a better name rather than text-variable-anchor if this property affects icons too.. 💭 )

I might rephrase as:

Fix text-variable-anchor not trying multiple placements during collision detection for icons when icon-text-fit is enabled.

CHANGELOG.md Outdated
* `hash` Map option can now be set as a string which sets the map hash to a custom query parameter. ([#8603](https://github.com/mapbox/mapbox-gl-js/pull/8603)) (h/t [SebCorbin](https://github.com/SebCorbin))

## 🍏 Improvements
* `icon-text-fit` now properly respects `text-writing-mode`:`vertical` ([#8835](https://github.com/mapbox/mapbox-gl-js/pull/8835))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: not really sure if there's a rule here, but I have a slight preference for "Add support for icon-text-fit with vertical labels."

Also I'm thinking whether this is an improvement or bug fix...?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tis a bug fix

CHANGELOG.md Outdated
## 🍏 Improvements
* `icon-text-fit` now properly respects `text-writing-mode`:`vertical` ([#8835](https://github.com/mapbox/mapbox-gl-js/pull/8835))
* GeolocateControl emits an `outofmaxbounds` and cancels the geolocate `flyTo` if the user is outside of `map.maxBounds` ([#8756](https://github.com/mapbox/mapbox-gl-js/pull/8756)) (h/t [MoradiDavijani](https://github.com/MoradiDavijani))
* Symbols fade faster when zooming out quickly, reducing overlap. ([#8628](https://github.com/mapbox/mapbox-gl-js/pull/8628))
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: if we want to preserve consistency in starting with a verb, "Fade symbols faster..."

@arindam1993 arindam1993 merged commit 48d1b92 into release-sangria Oct 17, 2019
@arindam1993 arindam1993 deleted the 1.5.0-beta.1 branch October 17, 2019 20:47
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.

5 participants