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

fix: removed underscore from isLabel_ in flyout_button.ts #7533

Merged
merged 3 commits into from
Sep 22, 2023
Merged

fix: removed underscore from isLabel_ in flyout_button.ts #7533

Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0228ecd
Removed underscore from isLabel_ in flyout_button.ts
sayali-kandarkar Sep 22, 2023
eb2651d
chore: rename isLabel_ to isFlyoutLabel in flyout_button.ts
sayali-kandarkar Sep 22, 2023
6b70fa7
fix: format
sayali-kandarkar Sep 22, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 13 additions & 13 deletions core/flyout_button.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,14 +59,14 @@ export class FlyoutButton {
* @param workspace The workspace in which to place this button.
* @param targetWorkspace The flyout's target workspace.
* @param json The JSON specifying the label/button.
* @param isLabel_ Whether this button should be styled as a label.
* @param isLabel Whether this button should be styled as a label.
* @internal
*/
constructor(
private readonly workspace: WorkspaceSvg,
private readonly targetWorkspace: WorkspaceSvg,
json: toolbox.ButtonOrLabelInfo,
private readonly isLabel_: boolean,
private readonly isLabel: boolean,
) {
this.text = json['text'];

Expand All @@ -93,7 +93,7 @@ export class FlyoutButton {
* @returns The button's SVG group.
*/
createDom(): SVGElement {
let cssClass = this.isLabel_ ? 'blocklyFlyoutLabel' : 'blocklyFlyoutButton';
let cssClass = this.isLabel ? 'blocklyFlyoutLabel' : 'blocklyFlyoutButton';
if (this.cssClass) {
cssClass += ' ' + this.cssClass;
}
Expand All @@ -105,7 +105,7 @@ export class FlyoutButton {
);

let shadow;
if (!this.isLabel_) {
if (!this.isLabel) {
// Shadow rectangle (light source does not mirror in RTL).
shadow = dom.createSvgElement(
Svg.RECT,
Expand All @@ -123,7 +123,7 @@ export class FlyoutButton {
const rect = dom.createSvgElement(
Svg.RECT,
{
'class': this.isLabel_
'class': this.isLabel
? 'blocklyFlyoutLabelBackground'
: 'blocklyFlyoutButtonBackground',
'rx': FlyoutButton.BORDER_RADIUS,
Expand All @@ -135,7 +135,7 @@ export class FlyoutButton {
const svgText = dom.createSvgElement(
Svg.TEXT,
{
'class': this.isLabel_ ? 'blocklyFlyoutLabelText' : 'blocklyText',
'class': this.isLabel ? 'blocklyFlyoutLabelText' : 'blocklyText',
'x': 0,
'y': 0,
'text-anchor': 'middle',
Expand All @@ -148,7 +148,7 @@ export class FlyoutButton {
text += '\u200F';
}
svgText.textContent = text;
if (this.isLabel_) {
if (this.isLabel) {
this.svgText = svgText;
this.workspace
.getThemeManager()
Expand All @@ -172,7 +172,7 @@ export class FlyoutButton {
);
this.height = fontMetrics.height;

if (!this.isLabel_) {
if (!this.isLabel) {
this.width += 2 * FlyoutButton.TEXT_MARGIN_X;
this.height += 2 * FlyoutButton.TEXT_MARGIN_Y;
shadow?.setAttribute('width', String(this.width));
Expand Down Expand Up @@ -227,8 +227,8 @@ export class FlyoutButton {
}

/** @returns Whether or not the button is a label. */
isLabel(): boolean {
return this.isLabel_;
isButtonLabel(): boolean {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Aha, this must be why we didn't make this change before. The isLabel method is public, so we don't want to rename it.

Instead, please change isLabel_ -> isFlyoutLabel.

Sorry for the wrong instructions!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hey Rachel,

Thank you so much for your review.
I made the necessary changes and pushed them.
Could you please take a look once?

Thanks,
Sayali

return this.isLabel;
}

/**
Expand Down Expand Up @@ -279,19 +279,19 @@ export class FlyoutButton {
gesture.cancel();
}

if (this.isLabel_ && this.callbackKey) {
if (this.isLabel && this.callbackKey) {
console.warn(
'Labels should not have callbacks. Label text: ' + this.text,
);
} else if (
!this.isLabel_ &&
!this.isLabel &&
!(
this.callbackKey &&
this.targetWorkspace.getButtonCallback(this.callbackKey)
)
) {
console.warn('Buttons should have callbacks. Button text: ' + this.text);
} else if (!this.isLabel_) {
} else if (!this.isLabel) {
const callback = this.targetWorkspace.getButtonCallback(this.callbackKey);
if (callback) {
callback(this);
Expand Down
Loading