Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Enable/disable location share button when setting is changed #7545

Merged
merged 1 commit into from
Jan 14, 2022

Conversation

andybalaam
Copy link
Member

@andybalaam andybalaam commented Jan 14, 2022

immediate-loc-share


This PR currently has no changelog labels, so will not be included in changelogs.

A reviewer can add one of: T-Deprecation, T-Enhancement, T-Defect, T-Task to indicate what type of change this is, or add Type: [enhancement/defect/task] to the description and I'll add them for you.

Preview: https://61e185e16a0e260b6b33d67a--matrix-react-sdk.netlify.app
⚠️ Do you trust the author of this PR? Maybe this build will steal your keys or give you malware. Exercise caution. Use test accounts.

@andybalaam andybalaam requested a review from a team as a code owner January 14, 2022 14:13
Copy link
Member

@t3chguy t3chguy left a comment

Choose a reason for hiding this comment

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

LGTM otherwise

Comment on lines +352 to +353
const showLocationButton = SettingsStore.getValue(
"feature_location_share");
Copy link
Member

Choose a reason for hiding this comment

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

this looks like it would fit on one line

Copy link
Member Author

Choose a reason for hiding this comment

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

Based on "try to keep JavaScript code around the 90 column mark" from the style doc (this is 100 columns), I'd vote for wrapping it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Of course, if I had my way we'd have a hard limit at 80 columns except imports and JSX...

@andybalaam andybalaam merged commit ae2cb63 into develop Jan 14, 2022
@andybalaam andybalaam deleted the psf-549-immediately-enable-location-button branch January 14, 2022 15:04
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants