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

feat(module: table): Add Align property for column #2045

Merged
merged 3 commits into from
Oct 22, 2021

Conversation

Qyperion
Copy link
Contributor

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • Bundle size optimization
  • Performance optimization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Other (about what?)

🔗 Related issue link

💡 Background and solution

📝 Changelog

Language Changelog
🇺🇸 English
🇨🇳 Chinese

☑️ Self Check before Merge

⚠️ Please check all items below before review. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • Changelog is provided or not needed

@github-actions
Copy link

github-actions bot commented Oct 19, 2021

@codecov
Copy link

codecov bot commented Oct 20, 2021

Codecov Report

Merging #2045 (c68b4d4) into master (b459dcc) will increase coverage by 1.91%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2045      +/-   ##
==========================================
+ Coverage   25.51%   27.42%   +1.91%     
==========================================
  Files         488      512      +24     
  Lines       32193    24562    -7631     
  Branches        0      238     +238     
==========================================
- Hits         8214     6737    -1477     
+ Misses      23979    17789    -6190     
- Partials        0       36      +36     
Impacted Files Coverage Δ
components/core/Helpers/CssStyleBuilder.cs 0.00% <0.00%> (ø)
components/table/Column.razor 0.00% <ø> (ø)
components/table/ColumnBase.cs 0.00% <0.00%> (ø)
components/core/Helpers/MemberPath/PathNode.cs 46.15% <0.00%> (-8.85%) ⬇️
components/core/Reflection/TypeDefined.cs 77.77% <0.00%> (-7.94%) ⬇️
components/input/InputGroup.razor.cs 87.50% <0.00%> (-6.95%) ⬇️
components/tabs/TabPane.razor 86.66% <0.00%> (-6.20%) ⬇️
...ponents/date-picker/types/DatePickerPlaceholder.cs 22.72% <0.00%> (-4.20%) ⬇️
components/input/TextArea.razor.cs 77.52% <0.00%> (-3.05%) ⬇️
...ponents/core/Component/Overlay/type/TriggerType.cs 81.25% <0.00%> (-2.97%) ⬇️
... and 424 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b459dcc...c68b4d4. Read the comment docs.

@ElderJames ElderJames linked an issue Oct 20, 2021 that may be closed by this pull request
{
string alignStyle = Align switch
{
ColumnAlign.Left => "text-align: left",
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this switch option is never used due to the condition in line 156.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But since it confused you, I deleted ColumnAlign.Left case.

private string _fixedStyle;

protected string FixedStyle => _fixedStyle;
protected string FixedStyle { get; private set; }
Copy link
Member

Choose a reason for hiding this comment

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

Why did you delete the _fixedStyle field?

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 decided to replace private field _fixedStyle and protected get-only FixedStyle property by one auto-property with protected get and private set. In terms of functionality, this is the same as it was before.
Do you have anything against this?

Copy link
Member

Choose a reason for hiding this comment

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

IMO, I think we no need to change this field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok. I reverted these changes.

@ElderJames ElderJames merged commit 08b02c5 into ant-design-blazor:master Oct 22, 2021
ElderJames pushed a commit that referenced this pull request Apr 23, 2022
* feat(module: table): Add Align property

* feat(module: table): Add Align property

* feat(module: table): Add Align property
ElderJames pushed a commit that referenced this pull request Apr 30, 2022
* feat(module: table): Add Align property

* feat(module: table): Add Align property

* feat(module: table): Add Align property
ElderJames pushed a commit that referenced this pull request Sep 6, 2022
* feat(module: table): Add Align property

* feat(module: table): Add Align property

* feat(module: table): Add Align property
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Table: columns alignment
3 participants