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

Unnecessary variable ShowLogo in Email Accounts Page #2422

Merged
merged 1 commit into from
Dec 2, 2024
Merged

Unnecessary variable ShowLogo in Email Accounts Page #2422

merged 1 commit into from
Dec 2, 2024

Conversation

StefanSosic
Copy link
Contributor

@StefanSosic StefanSosic commented Nov 29, 2024 •
edited by github-actions bot
Loading

Summary

Variable Show Logo on Email Accounts page have no function:
image

image

It's always set to true.
We could do cleanup and remove it, behavior would stay same.

Work Item(s)

Fixes #2365

Fixes AB#559267

@StefanSosic StefanSosic requested a review from a team as a code owner November 29, 2024 20:56
@github-actions github-actions bot added AL: System Application From Fork Pull request is coming from a fork labels Nov 29, 2024
JesperSchulz
JesperSchulz approved these changes Dec 2, 2024
View reviewed changes
Copy link
Contributor

@JesperSchulz JesperSchulz left a comment

Choose a reason for hiding this comment

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

Yes, indeed pointless.

@JesperSchulz JesperSchulz added Integration GitHub request for Integration area Linked Issue is linked to a Azure Boards work item labels Dec 2, 2024
@github-actions github-actions bot added this to the Version 26.0 milestone Dec 2, 2024
@NKarolak
Copy link
Contributor

NKarolak commented Dec 2, 2024

Is there any scenario where it would make sense to hide the logo by default?
If yes, then the variable should not be removed, but a setter function should be added instead (one to set it to false. If not used, the value will still be set to true by default).

In any case, removing Visible = ShowLogo; from the repeater control (as suggested) seems to be correct.

@JesperSchulz JesperSchulz enabled auto-merge (squash) December 2, 2024 15:38
@JesperSchulz JesperSchulz merged commit 31bef45 into microsoft:main Dec 2, 2024
27 of 28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@JesperSchulz JesperSchulz JesperSchulz approved these changes

@grobyns grobyns grobyns approved these changes

Assignees
No one assigned
Labels
AL: System Application From Fork Pull request is coming from a fork Integration GitHub request for Integration area Linked Issue is linked to a Azure Boards work item
Projects
None yet
Milestone
Version 26.0
Development

Successfully merging this pull request may close these issues.

[Bug]: Unnecessary variabl ShowLogo in Email Accounts Page
4 participants
@StefanSosic @NKarolak @JesperSchulz @grobyns