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

Changes for print_form_button() #1109

Merged
merged 4 commits into from Sep 9, 2017
Merged

Conversation

cproensa
Copy link
Contributor

Some proposals for buttons generated from print_form_button(), and buttons layout in general.

  • Use button tag, instead of input, as the former can include icons. We use intensively these buttons in manage pages, one or several in each row of a table. Having the full text for all buttons add clutter to the page, so at some point they could be changed to icons
  • Buttons generated from print_form_button() are not inlined by themselves, as they are placed as standalone forms, which have block display style. This does not correspond to the behavoiur of standard button or inputs, which are inline elements by default.
  • Buttons generated from print_form_button() does not show spacing between them, whereas standard buttons in a form do have some margin.
  • Experiment with a visibility toggle for some manage buttons, when these buttons are used for actions on every row in a table. By making them visible only when hovering on its relevant container, the page may look cleaner.

@cproensa
Copy link
Contributor Author

Before: configuration report page

seleccion_191

After adding margin

seleccion_193

After adding visibility on hover (row):

seleccion_192

Copy link
Member

@dregad dregad left a comment

Choose a reason for hiding this comment

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

You have opened several related issues on the tracker, but forgot to add the Fixes #xxx references in the commit messages.

Other than that, it looks good to me.

@cproensa
Copy link
Contributor Author

cproensa commented May 16, 2017

You have opened several related issues on the tracker, but forgot to add the Fixes #xxx references in the commit messages.

yes.. as i see the changes are likely to be discussed and changed, dind't put the "fixes..." right away.
I'd wait for feedback on the topics, especially @syncguru

Copy link
Contributor

@syncguru syncguru left a comment

Choose a reason for hiding this comment

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

Other the my comment, this looks good.

@@ -199,7 +199,7 @@
?>
<div class="clearfix"></div>
<div class="space-2"></div>
<div class="btn-group-sm">
<div class="btn-group visible-on-hover">
Copy link
Contributor

Choose a reason for hiding this comment

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

It is not very clear to me if removing "-sm" is intentional. If so, why?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

two reasons:

  • other buttons in the pages are within plain "btn-group"
  • to make it apply the margins with the defined css selector: .btn-group .single-button-form .
    Otherwise, the selector should be repeated for every .btn-group-xx

What do you think?

@cproensa cproensa force-pushed the print_form_button branch 2 times, most recently from 5a7d880 to 59f9f11 Compare August 10, 2017 23:21
@cproensa
Copy link
Contributor Author

You have opened several related issues on the tracker, but forgot to add the Fixes #xxx references in the commit messages.

Rebased, and added the bug references in the commits

Use 'button' tag instead of 'input', to offer better customization for
labels and icons.
Add inline class to single button forms.
Now it should not be needed to "pull-left" to place several buttons in
line.

Fixes: #22871
Add margin between buttons generated by print_form_button(), to be
consistent with the general styling of inline buttons in a general form.

Fixes: #22870
Make some buttons visible only when hovering over it's container.

Applied to: adm_config_report.php, view.php (bugnotes)

Fixes: #22872
@dregad dregad merged commit b0c652f into mantisbt:master Sep 9, 2017
dregad added a commit that referenced this pull request Sep 9, 2017
@cproensa cproensa deleted the print_form_button branch March 31, 2019 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants