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

Add IsSublte and HorizontalAlignment to Element #255

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

codello
Copy link

@codello codello commented Feb 19, 2024

This PR adds two fields to the Element struct:

  • IsSubtle: For setting subtle text appearance
  • HorizontalAlignment: For setting the horizontal alignment of text elements

Reference: https://adaptivecards.io/explorer/TextBlock.html

I reused the horizontal alignment values from Column.HorizontalCellContentAlignment as the values are the same and I felt that having something like HorizontalTextAlignmentLeft would cause confusion when the field itself is called HorizontalAlignment.

Closes #254

@atc0005
Copy link
Owner

atc0005 commented Feb 20, 2024

I reused the horizontal alignment values from Column.HorizontalCellContentAlignment as the values are the same and I felt that having something like HorizontalTextAlignmentLeft would cause confusion when the field itself is called HorizontalAlignment.

Since that helper function is unexported we could take a couple of approaches:

  1. rename supportedHorizontalContentAlignmentValues to supportedHorizontalAlignmentValues
    • also update the doc comment to remove the word content since that was specific to the Table type
  2. add a second helper function named something like supportedHorizontalTextAlignmentValues
    • this function could either call the other function (e.g., return supportedHorizontalAlignmentValues()) or duplicate the slice of constants
    • having this function might make clear that there is a distinction between TextBlock and Table alignment values (assuming there is a distinction)

I'm in favor of option 1 if we're looking to reduce duplication.

@codello
Copy link
Author

codello commented Feb 21, 2024

rename supportedHorizontalContentAlignmentValues to supportedHorizontalAlignmentValues

I also prefer that option. I updated the PR.

@atc0005 atc0005 self-requested a review February 21, 2024 23:14
@atc0005 atc0005 self-assigned this Feb 21, 2024
@atc0005 atc0005 added enhancement New feature or request card format/adaptivecard Adaptive Card support labels Feb 21, 2024
@atc0005 atc0005 added this to the v2.9.1 milestone Feb 21, 2024
@atc0005
Copy link
Owner

atc0005 commented Feb 21, 2024

@codello Thanks!

Are you comfortable squashing your commits?

@codello
Copy link
Author

codello commented Feb 22, 2024

Sure, done.

Copy link
Owner

@atc0005 atc0005 left a comment

Choose a reason for hiding this comment

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

Thank you for all of your work on this!

@atc0005 atc0005 merged commit 84950e7 into atc0005:master Feb 22, 2024
30 of 31 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
card format/adaptivecard Adaptive Card support enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Setting IsSubtle and other fields
2 participants