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

Support configuring a form to read right to left via the properties panel #1205

Open
okaeiz opened this issue Jun 8, 2024 · 6 comments
Open
Assignees
Labels
backlog Queued in backlog enhancement New feature or request

Comments

@okaeiz
Copy link

okaeiz commented Jun 8, 2024

Is your feature request related to a problem? Please describe.

Yes. I am using form-js in Camunda Desktop Modeler. When the form is in Hebrew, Persian or Arabic, the placeholders, labels and values are rendered left to right. It is frustrating for RTL users. I request the RTL support for forms or form elements separately.

Describe the solution you'd like

I think there are 2 ways to achieve this:

  1. Add a direction option to the form properties.
  2. Add a direction option to every form element such as text field, text area, datetime, etc.

Describe alternatives you've considered

After the implementation of form-js HTML View element, the HTML tags for templating and representing data (such as h1, h2, a, li, etc.) could be modified and assigned CSS attributes. I managed to change the font and direction in HTML View. But apparently the use of JavaScript for passing input values in HTML View is disabled due to security reasons. In other words, no inputs can be used in HTML View. Therefore, I still have to use text field and text area which are not customizable.

Additional context

I have also raised issues in Camunda platform official repositories. The RTL support seems to be a good idea and it definitely is; because it attracts a large number of users who speak an RTL language to the community.

@okaeiz okaeiz added the enhancement New feature or request label Jun 8, 2024
@Skaiir
Copy link
Contributor

Skaiir commented Jun 10, 2024

So, in terms of how I see this implementation going:

The functionality should be provided through configuration, so you'd instantiate your form like this:

const form = new Form({ data, schema, i18n: { rtl: true } );

Then you can access this config via

const config = useService('config');
const { i18n } = config || {};
const { rtl = false };

and set some conditions on the base fields to define this rtl behavior.

Edit:

Nevermind this, it's only relevant in the context of external configuration, which we can tackle later. I'll specify the title of this issue.

@Skaiir Skaiir changed the title Right to Left support for elements Support configuring form fields to read right to left via the properties panel. Jun 11, 2024
@Skaiir Skaiir changed the title Support configuring form fields to read right to left via the properties panel. Support configuring form fields to read right to left via the properties panel Jun 11, 2024
@Skaiir Skaiir changed the title Support configuring form fields to read right to left via the properties panel Support configuring a form to read right to left via the properties panel Jun 11, 2024
@okaeiz
Copy link
Author

okaeiz commented Jun 15, 2024

@Skaiir Thank you. I played around with the code and this is what I achieved. I created a StyleGroup since the Appearance name was already used for another group. Then I added a DirectionEntry to the group using which I can select the direction.

Capture.mp4

As you can see, the Button component is rendered according to what the user has selected. I am implementing the same fetching logic for all the components. Based on the earlier discussions under this issue, I intend to add an extra entry to the Text Field component which can overwrite the global settings of direction for possible LTR values such as emails and URLs. Am I on the right track?

@Skaiir
Copy link
Contributor

Skaiir commented Jun 16, 2024

@okaeiz I would say, add your entry in the appearance group directly. The reason we bring all the properties in via groups like this is to (in theory, not sure it works at the moment 😏) preserve the group opening state across components, on the assumption that people may want to edit appearance related things in batches.

So you'd just have a check that does if type = default add something.

But yes you're on the right track. The PR needs to be cleaned up however, I expect maybe 1 or 2 commits, and not such a huge diff.

@okaeiz
Copy link
Author

okaeiz commented Jun 16, 2024

@Skaiir Thank you for the guidance. So if I'm not mistaken, I should do the following:

  1. Instead of a StyleGroup, I need to add the DirectionEntry to the existing AppearanceGroup.
  2. The AppearanceGroup need to only include the so-called DirectionEntry for the default type; not anything else. So when the user selects the form component, they'll see two groups: General (ID entry) and Appearance (Direction Entry)
  3. The PR shouldn't consist of more than 2 commits; and should be saved as Draft if is not complete.

Despite the implementation is not complete, I just created the PR in order to confirm I'm doing things right. I'll adhere to what you requested in the next PR. Moreover, I didn't fully comprehend what you meant by "such a huge diff". Do you think I should keep the whole scope smaller in a single PR?

@Skaiir
Copy link
Contributor

Skaiir commented Jun 16, 2024

@okaeiz Just look at the PR and the associated changes, there's -6000 and +2000 lines, this is unexpected and most likely due to the fact that your branch is weirdly synchronized (there's also a bunch of merge commits in there that are messy).

The 1/2 commits is just based on how I'd split it up for something this size, like maybe one commit for the properties panel changes, and then one commit for the button changes. You don't have to stick to it if you can split it in other ways that make sense, but right now it looks messy so it's hard for me to take a look at it. But basically, it should be clean once you're ready for me to review it. Until then do what you like, but keep it in draft so we know you're not done ;).

But you're pointing to the develop branch, which is the correct thing to do for new functionality. If there's anything else let me know. You are heading the right direction from what I'm seeing.

@okaeiz
Copy link
Author

okaeiz commented Jun 16, 2024

@Skaiir I highly appreciate your help. I'll let you know if I had any further questions. 🤝

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog Queued in backlog enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants