Skip to content
This repository has been archived by the owner on Jun 7, 2022. It is now read-only.

Fix date range dropdown for RTL locales #1803

Merged
merged 2 commits into from Mar 18, 2019
Merged

Conversation

Aljullu
Copy link
Contributor

@Aljullu Aljullu commented Mar 14, 2019

Fixes second issue from #1776.

  • Fixes date range dropdown alignment for RTL locales.
  • Fixes calendar error tooltips positioning, until now they were not visible neither in RTL nor LTR locales.

Screenshots

Before:
image

After:
image

Detailed test instructions:

  • Using a RTL language, go to any report.
  • Open the date picker dropdown.
  • Verify the dropdown is correctly aligned.
  • Enter an invalid date.
  • Verify the 'Invalid Date' popover is correctly positioned.
  • Bonus points for testing there are no regressions in mobile and LTR languages.

@Aljullu Aljullu added [Status] Needs Review focus: components Issues for woocommerce components labels Mar 14, 2019
@Aljullu Aljullu self-assigned this Mar 14, 2019
@Aljullu Aljullu requested a review from a team March 14, 2019 12:46
@Aljullu Aljullu mentioned this pull request Mar 14, 2019
18 tasks
left: 50% !important;
position: absolute;
top: auto !important;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Using Chrome and looking at a RTL language, the left inline style doesn't get overwritten:

Screen Shot 2019-03-15 at 9 11 04 AM

It looks like the left: 50% !important; gets rewritten by the rtl plugin to right: 50% !important;. If I change it back to left: 50% !important; in the console, it works.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right! I tested it in Firefox and wrongly assumed it would work in other browsers too.

I made that CSS ruleset to be ignored by the RTL plugin, so it should work fine now in both browsers.

Copy link
Collaborator

@psealock psealock left a comment

Choose a reason for hiding this comment

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

I'm not sure if this is a browser issue, but I noted an oddity with the left/right centering of the tooltip error message.

Thanks for tackling this! It works in non RTL as well. My calendar isn't rendering either, could that be related?

@Aljullu
Copy link
Contributor Author

Aljullu commented Mar 15, 2019

Thanks for the review @psealock. Can you take another look now? 🙂

My calendar isn't rendering either, could that be related?

Right, that's the last remaining issue from #1776. I'm trying to figure out what's going on there. I guess some CSS is interfering with react-dates, but I couldn't find yet what it is, so I don't know if both issues are related or they are something completely different.

Copy link
Collaborator

@psealock psealock left a comment

Choose a reason for hiding this comment

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

LGTM!

@Aljullu Aljullu merged commit aae8359 into master Mar 18, 2019
@Aljullu Aljullu added this to Sprint 14 Done in Isotope via automation Mar 18, 2019
@Aljullu Aljullu deleted the fix/rtl-date-dropdown branch March 18, 2019 09:47
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
focus: components Issues for woocommerce components
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants