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.

Sign up for GitHub

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

Jump to bottom

Unclear default visibility for loginForm elements 3286 #3328

Merged
merged 13 commits into from
Jul 9, 2024
Merged

Unclear default visibility for loginForm elements 3286 #3328

merged 13 commits into from
Jul 9, 2024

Conversation

ilushaaa
Copy link
Contributor

@ilushaaa ilushaaa commented Jun 3, 2024

@ilushaaa ilushaaa requested a review from glebfox June 3, 2024 11:08
@ilushaaa ilushaaa linked an issue Jun 3, 2024 that may be closed by this pull request
Unclear default visibility for loginForm elements #3286
Closed
@knstvk knstvk changed the title Unclear default visibility for loginFrom elements 3286 Unclear default visibility for loginForm elements 3286 Jun 4, 2024
@ilushaaa ilushaaa requested a review from glebfox June 4, 2024 13:29
@glebfox glebfox requested review from Flaurite and glebfox and removed request for glebfox June 6, 2024 10:43
@ilushaaa ilushaaa requested a review from Flaurite June 10, 2024 07:34
@glebfox glebfox removed the request for review from Flaurite June 10, 2024 10:05
@ilushaaa ilushaaa requested a review from glebfox June 11, 2024 13:15
glebfox
glebfox requested changes Jun 14, 2024
View reviewed changes
...lowui/flowui-kit/src/main/java/io/jmix/flowui/kit/component/loginform/EnhancedLoginForm.java Outdated Show resolved Hide resolved
...flowui/src/main/java/io/jmix/flowui/xml/layout/loader/component/AbstractLoginFormLoader.java Outdated Show resolved Hide resolved
jmix-flowui/flowui-kit/src/main/java/io/jmix/flowui/kit/meta/component/StudioComponents.java
@StudioProperty(xmlAttribute = "enabled", type = StudioPropertyType.BOOLEAN,
defaultValue = "true"),
@StudioProperty(xmlAttribute = "localesVisible", type = StudioPropertyType.BOOLEAN),
@StudioProperty(xmlAttribute = "localesVisible", type = StudioPropertyType.BOOLEAN,
defaultValue = "false"),
Copy link
Contributor

Choose a reason for hiding this comment

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

it's still visible, even if true is not set explicitly.

When you fix it, the other problem will appear that you need to set it to true explicitly, otherwise all existing views will be broken. Let's set it to true if io.jmix.flowui.kit.component.loginform.EnhancedLoginForm#setLocaleItems with not empty value to avoid breaking changes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we set localesVisible to true depending on the number of available locales - do we need this property at all?

loginForm.localesSelect.hidden = locales.length > 1 ? false : true;

The locales select with single item doesn't have any matter.

jmix-flowui/flowui-kit/src/main/java/io/jmix/flowui/kit/component/loginform/JmixLoginI18n.java Outdated Show resolved Hide resolved
jmix-flowui/flowui-kit/src/main/java/io/jmix/flowui/kit/component/loginform/JmixLoginI18n.java Show resolved Hide resolved
@ilushaaa ilushaaa requested a review from glebfox June 21, 2024 09:39
@ilushaaa ilushaaa requested a review from Flaurite June 24, 2024 10:26
@ilushaaa ilushaaa requested a review from glebfox July 9, 2024 09:10
@ilushaaa ilushaaa merged commit 92df743 into master Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@glebfox glebfox glebfox approved these changes

@Flaurite Flaurite Awaiting requested review from Flaurite

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Unclear default visibility for loginForm elements
3 participants
@ilushaaa @glebfox @Flaurite