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(Listbox): rtl mode #1144

Merged
merged 9 commits into from
Mar 16, 2023
Merged

fix(Listbox): rtl mode #1144

merged 9 commits into from
Mar 16, 2023

Conversation

PurwaShrivastava
Copy link
Contributor

@PurwaShrivastava PurwaShrivastava commented Mar 14, 2023

Motivation

RTL mode for the ListBox. This is to fix qlik-oss/sn-list-objects#82

Before: Title, Search Icon, Columns & Toolbar were not correctly aligned for Folded as well as normal ListBox.
Number Alignment was not correct & - sign for negative numbers shown backwards.

After: Attached Screenshots
Litbox Title, Search Glass & Columns with Numeric positive negative as well as String values in RTL mode :

Screenshot 2023-03-15 at 11 36 50

Actions Toolbar in RTL Mode :

Screenshot 2023-03-16 at 11 06 18

Folded ListBox in RTL Mode :

Screenshot 2023-03-15 at 11 37 20

Requirements checklist

  • Api specification
    • Ran yarn spec
      • No changes OR API changes has been formally approved
  • Unit/Component test coverage
  • Correct PR title for the changes (fix, chore, feat)

When build and tests have passed:

  • Add code reviewers, for example @qlik-oss/nebula-core

@PurwaShrivastava PurwaShrivastava requested review from a team, T-Wizard, johanlahti, DanielS-Qlik and quanho and removed request for a team, johanlahti, T-Wizard, quanho and DanielS-Qlik March 14, 2023 20:27
@PurwaShrivastava PurwaShrivastava changed the title fix(Listbox): rtl mode WIP : fix(Listbox): rtl mode Mar 15, 2023
@PurwaShrivastava PurwaShrivastava changed the title WIP : fix(Listbox): rtl mode fix(Listbox): rtl mode Mar 15, 2023
@quanho quanho requested a review from Caele March 16, 2023 11:40
@Caele
Copy link
Collaborator

Caele commented Mar 16, 2023

  • Look at adding a rendering test to cover the RTL case
  • The current rendering tests should not change or?

I have updated the snapshots for rendering tests, have added unit tests for RTL case. Can look into adding rendering tests.

@Caele
Copy link
Collaborator

Caele commented Mar 16, 2023

I have updated the snapshots for rendering tests, have added unit tests for RTL case. Can look into adding rendering tests.

I meant that the changes you have made should not affect the current rendering tests, so was there an intentional style update there somewhere or?

@PurwaShrivastava
Copy link
Contributor Author

I have updated the snapshots for rendering tests, have added unit tests for RTL case. Can look into adding rendering tests.

I meant that the changes you have made should not affect the current rendering tests, so was there an intentional style update there somewhere or?

I have updated the snapshots for rendering tests, have added unit tests for RTL case. Can look into adding rendering tests.

I meant that the changes you have made should not affect the current rendering tests, so was there an intentional style update there somewhere or?

Have introduced a right padding for header.

@PurwaShrivastava PurwaShrivastava merged commit b25bddc into master Mar 16, 2023
@PurwaShrivastava PurwaShrivastava deleted the atq/rtl branch March 16, 2023 20:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RTL: Right to left does not work fully
3 participants