Skip to content

Fix 3776 L&F review for user/group form#3804

Merged
offtherailz merged 5 commits intogeosolutions-it:masterfrom
allyoucanmap:gia8789_fix_3776
May 27, 2019
Merged

Fix 3776 L&F review for user/group form#3804
offtherailz merged 5 commits intogeosolutions-it:masterfrom
allyoucanmap:gia8789_fix_3776

Conversation

@allyoucanmap
Copy link
Copy Markdown
Contributor

@allyoucanmap allyoucanmap commented May 24, 2019

Description

L&F review for user/group form

Issues

Please check if the PR fulfills these requirements

What kind of change does this PR introduce? (check one with "x", remove the others)

  • Other... Please describe: Improvement styles

What is the current behavior? (You can also link to an open issue here)
#3776

What is the new behavior?
Updated look and feel of User and Group Dialogs:

  • centered dialogs
  • added * for mandatory fields
  • removed buttons from nav in favor of tabs pills

Does this PR introduce a breaking change? (check one with "x", remove the other)

  • No

If this PR contains a breaking change, please describe the impact and migration path for existing applications: ...

Other information:

@allyoucanmap allyoucanmap added this to the 2019.02.00 milestone May 24, 2019
@allyoucanmap allyoucanmap mentioned this pull request May 24, 2019
Closed
5 tasks
@allyoucanmap allyoucanmap requested a review from offtherailz May 24, 2019 15:56
@coveralls
Copy link
Copy Markdown

coveralls commented May 24, 2019

Coverage Status

Coverage decreased (-0.04%) to 81.378% when pulling 7da6da8 on allyoucanmap:gia8789_fix_3776 into cc51371 on geosolutions-it:master.

offtherailz
offtherailz reviewed May 27, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@offtherailz offtherailz left a comment •
edited
Loading

Choose a reason for hiding this comment

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

It's ok. The only thing I noticed, that is strange, is that there is a limit of 20 chars for the group name (on db is 255, don't know why you have an error from the server when you try to add more than 20.

Anyway you are allowed to insert more than 20 char in the group name, and when saving ,you have an error, not well specified.
Maybe we can open another issue to investigate and fix this specific issue. @tdipisa what do you think?

Another small thing I noticed. Is this name duplication still required? The same for username
image

@tdipisa
Copy link
Copy Markdown
Member

tdipisa commented May 27, 2019

It's ok. The only thing I noticed, that is strange, is that there is a limit of 20 chars for the group name (on db is 255, don't know why you have an error from the server when you try to add more than 20.

Anyway you are allowed to insert more than 20 char in the group name, and when saving ,you have an error, not well specified.
Maybe we can open another issue to investigate and fix this specific issue. @tdipisa what do you think?

@offtherailz open a new issue for this

Another small thing I noticed. Is this name duplication still required? The same for username

@offtherailz yes, we have decided to maintain it as it is for the moment

Beside the above, if all is ok please approve and merge

@offtherailz offtherailz merged commit 7fb4e66 into geosolutions-it:master May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@offtherailz offtherailz offtherailz approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

2019.02.00

Development

Successfully merging this pull request may close these issues.

5 participants

@allyoucanmap @coveralls @tdipisa @offtherailz @gia8789