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

fix(module:space): nzSpaceItem margin in rtl #7801

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

akg2011-code
Copy link

  • Change 'margin-right' to 'margin-inline-end' and
    'margin-bottom' to 'margin-block-end' for support any direction

- Change 'margin-right' to 'margin-inline-end' and
'margin-bottom' to 'margin-block-end' for support any direction
@zorro-bot
Copy link

zorro-bot bot commented Jan 5, 2023

This preview will be available after the AzureCI is passed.

@codecov
Copy link

codecov bot commented Jan 5, 2023

Codecov Report

Merging #7801 (c1970d6) into master (285655a) will not change coverage.
The diff coverage is n/a.

❗ Current head c1970d6 differs from pull request most recent head cab1458. Consider uploading reports for the commit cab1458 to get more accurate results

@@           Coverage Diff           @@
##           master    #7801   +/-   ##
=======================================
  Coverage   92.08%   92.08%           
=======================================
  Files         502      502           
  Lines       16782    16782           
  Branches     2562     2562           
=======================================
  Hits        15454    15454           
+ Misses       1059     1056    -3     
- Partials      269      272    +3     
Impacted Files Coverage Δ
components/space/space.component.ts 100.00% <ø> (ø)
components/table/src/table-style.service.ts 94.82% <0.00%> (-1.73%) ⬇️
components/tabs/tab-nav-bar.component.ts 87.23% <0.00%> (-0.71%) ⬇️
components/table/src/table/tr-measure.component.ts 91.30% <0.00%> (+4.34%) ⬆️
components/table/src/table/tbody.component.ts 100.00% <0.00%> (+11.11%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@simplejason
Copy link
Member

simplejason commented Jan 9, 2023

I find margin-block-end only works on v87+ in caniuse, I'm not sure if this is the best solution.

@akg2011-code
Copy link
Author

I find margin-block-end only works on v87+ in caniuse, I'm not sure if this is the best solution.

What is the lowest version it should be?

@Laffery Laffery added Component: Space 🌐 caniuse blocked by browser compatibility labels Jul 25, 2024
@lppedd
Copy link
Contributor

lppedd commented Dec 14, 2024

What is the lowest version it should be?

I think having proper localization (which includes RTL design) is more important than supporting old browsers anyways.

@Laffery
Copy link
Collaborator

Laffery commented Dec 20, 2024

What is the lowest version it should be?

I think having proper localization (which includes RTL design) is more important than supporting old browsers anyways.

Nearly two years on, we can look at the latest figures on caniuse, both margin-inline-end and margin-block-end are supported by over 96% of browsers.
Besides, antd had already used margin-inline-end in their components https://github.com/search?q=repo%3Aant-design%2Fant-design+margin-inline-end&type=code.

I think it's time for us to import them in ng-zorro, WDYT? @lppedd

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants