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

Empty grids and grid rows should not take vertical space #2414

Closed
ebruchez opened this Issue Oct 1, 2015 · 14 comments

Comments

Projects
1 participant
@ebruchez
Collaborator

ebruchez commented Oct 1, 2015

We already do something to remove them in PDF (#2004), but not in the UI.

+1 from customer

@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez Mar 4, 2016

Collaborator

See duplicate #2583 which shows the issue for repeated rows in particular.

Collaborator

ebruchez commented Mar 4, 2016

See duplicate #2583 which shows the issue for repeated rows in particular.

@ebruchez

This comment has been minimized.

Show comment
Hide comment
Collaborator

ebruchez commented Mar 14, 2017

@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez Mar 16, 2017

Collaborator

We are not sure that it is realistic to do this in CSS 3, for example with margin collapsing.

A realistic solution is to do this within grid.xbl: for each control nested in the grid/grid row, obtain its name and, statically, create a relevance expression for the grid/grid row:

(
  for $bind-name in ('control-1-bind', 'control-2-bind', etc.)
  return relevant(bind($bind-name))
) = true()

Meaning that the enclosing group is relevant if at least one nested control is relevant.

Collaborator

ebruchez commented Mar 16, 2017

We are not sure that it is realistic to do this in CSS 3, for example with margin collapsing.

A realistic solution is to do this within grid.xbl: for each control nested in the grid/grid row, obtain its name and, statically, create a relevance expression for the grid/grid row:

(
  for $bind-name in ('control-1-bind', 'control-2-bind', etc.)
  return relevant(bind($bind-name))
) = true()

Meaning that the enclosing group is relevant if at least one nested control is relevant.

@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez Mar 16, 2017

Collaborator

What about:

  • repeated grids
  • sections
  • repeated sections
Collaborator

ebruchez commented Mar 16, 2017

What about:

  • repeated grids
  • sections
  • repeated sections
@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez Mar 16, 2017

Collaborator

Also, do we need to be able to control this in the Form Builder UI?

Collaborator

ebruchez commented Mar 16, 2017

Also, do we need to be able to control this in the Form Builder UI?

@ebruchez ebruchez changed the title from Empty grid rows should not take vertical space to Empty grids and grid rows should not take vertical space Mar 20, 2017

@ebruchez ebruchez added this to the 2017.1 milestone Mar 20, 2017

@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez Mar 20, 2017

Collaborator

I prototyped that we can nicely hide grids and rows in the non-repeated mode.

In phone mode (single-column), table "cells" can still show space, because the layout looks like this:

<div class="fr-grid-td">
    <div class="fr-grid-content">
        <span id="section-1-control≡xf-395≡control-4-control" class="fr-grid-1-2 xforms-control xforms-input">
  • fr-grid-td"
    • doesn't have padding right now
    • has vertical alignment
  • fr-grid-content
    • has padding
    • used more in Form Builder?
  • xforms-control
    • has xforms-disabled when non-relevant

How can we handle this case?

Collaborator

ebruchez commented Mar 20, 2017

I prototyped that we can nicely hide grids and rows in the non-repeated mode.

In phone mode (single-column), table "cells" can still show space, because the layout looks like this:

<div class="fr-grid-td">
    <div class="fr-grid-content">
        <span id="section-1-control≡xf-395≡control-4-control" class="fr-grid-1-2 xforms-control xforms-input">
  • fr-grid-td"
    • doesn't have padding right now
    • has vertical alignment
  • fr-grid-content
    • has padding
    • used more in Form Builder?
  • xforms-control
    • has xforms-disabled when non-relevant

How can we handle this case?

@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez Mar 20, 2017

Collaborator

It would be nice to remove fr-grid-content which duplicates fr-grid-td. Is it there for historical reasons?

Collaborator

ebruchez commented Mar 20, 2017

It would be nice to remove fr-grid-content which duplicates fr-grid-td. Is it there for historical reasons?

@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez Mar 20, 2017

Collaborator

Can we:

  • remove fr-grid-content (at least at runtime)
  • replace the padding of fr-grid-content with a margin on the contained control?
Collaborator

ebruchez commented Mar 20, 2017

Can we:

  • remove fr-grid-content (at least at runtime)
  • replace the padding of fr-grid-content with a margin on the contained control?
@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez Mar 20, 2017

Collaborator

If an fr-grid-td doesn't take any space (no padding), then do we need relevance on fr-grid-tr and fr-grid?

Collaborator

ebruchez commented Mar 20, 2017

If an fr-grid-td doesn't take any space (no padding), then do we need relevance on fr-grid-tr and fr-grid?

@ebruchez

This comment has been minimized.

Show comment
Hide comment
Collaborator

ebruchez commented Mar 27, 2017

ebruchez added a commit that referenced this issue Mar 29, 2017

@ebruchez ebruchez modified the milestones: 2017.2, 2017.1 Apr 24, 2017

@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez May 17, 2017

Collaborator

Usages of fr-grid-content:

form-builder.less  (5 usages found)
    526td:nth-of-type(5) .fr-grid-content {
    727.fr-grid .fr-grid-content .xforms-trigger .btn input,
    728.fr-grid .fr-grid-content .xforms-trigger .fb-mock-link input
    760.fr-grid-content .xforms-label, .fr-grid-content .xforms-hint,
    764.fr-grid-content {
form-runner-base.less  (2 usages found)
    338.fr-grid .fr-grid-tr .fr-grid-content {
    380.fr-grid-content .xforms-control, .fr-grid-content .fr-component-group { margin-top: 0 }
form.xhtml  (1 usage found)
    31.fr-grid-content .xforms-control, .fr-grid-content .fr-component-group, .fr-grid-content span.xbl-fr-us-state {
grid.less  (6 usages found)
    180.fr-grid-content {
    186.fr-grid-content {
    193.fr-grid-content {
    201&> .fr-grid-content > * {
    277.fr-grid-content {
    281.fr-repeat .fr-grid-content {

action-icons.coffee  (3 usages found)
    24isNotEmpty = (gridTd) -> $(gridTd).find('.fr-grid-content').children().length > 0
    25isUpload = (gridTd) -> isNotEmpty(gridTd) and $(gridTd).find('.fr-grid-content > .xbl-component').hasClass('fb-upload')
    48'fb-edit-items-trigger':        (gridTd) -> isNotEmpty(gridTd) and $(gridTd).find('.fr-grid-content').hasClass('fb-itemset')
dialog-actions.xml  (1 usage found)
    128<xh:div class="fr-grid-content">
dialog-help.xbl  (1 usage found)
    78<xh:div class="fr-grid-content">
grid.xbl  (2 usages found)
    793<xh:div class="fr-grid-content">
    824<xh:div class="fr-grid-content{
placeholders.coffee  (2 usages found)
    31selector: '.fr-grid-content > .xforms-trigger-appearance-full'
    45selector: '.fr-grid-content > .xforms-trigger-appearance-minimal'
print-pdf-notemplate.xpl  (1 usage found)
    167                                    p:has-class('fr-grid-content')
Collaborator

ebruchez commented May 17, 2017

Usages of fr-grid-content:

form-builder.less  (5 usages found)
    526td:nth-of-type(5) .fr-grid-content {
    727.fr-grid .fr-grid-content .xforms-trigger .btn input,
    728.fr-grid .fr-grid-content .xforms-trigger .fb-mock-link input
    760.fr-grid-content .xforms-label, .fr-grid-content .xforms-hint,
    764.fr-grid-content {
form-runner-base.less  (2 usages found)
    338.fr-grid .fr-grid-tr .fr-grid-content {
    380.fr-grid-content .xforms-control, .fr-grid-content .fr-component-group { margin-top: 0 }
form.xhtml  (1 usage found)
    31.fr-grid-content .xforms-control, .fr-grid-content .fr-component-group, .fr-grid-content span.xbl-fr-us-state {
grid.less  (6 usages found)
    180.fr-grid-content {
    186.fr-grid-content {
    193.fr-grid-content {
    201&> .fr-grid-content > * {
    277.fr-grid-content {
    281.fr-repeat .fr-grid-content {

action-icons.coffee  (3 usages found)
    24isNotEmpty = (gridTd) -> $(gridTd).find('.fr-grid-content').children().length > 0
    25isUpload = (gridTd) -> isNotEmpty(gridTd) and $(gridTd).find('.fr-grid-content > .xbl-component').hasClass('fb-upload')
    48'fb-edit-items-trigger':        (gridTd) -> isNotEmpty(gridTd) and $(gridTd).find('.fr-grid-content').hasClass('fb-itemset')
dialog-actions.xml  (1 usage found)
    128<xh:div class="fr-grid-content">
dialog-help.xbl  (1 usage found)
    78<xh:div class="fr-grid-content">
grid.xbl  (2 usages found)
    793<xh:div class="fr-grid-content">
    824<xh:div class="fr-grid-content{
placeholders.coffee  (2 usages found)
    31selector: '.fr-grid-content > .xforms-trigger-appearance-full'
    45selector: '.fr-grid-content > .xforms-trigger-appearance-minimal'
print-pdf-notemplate.xpl  (1 usage found)
    167                                    p:has-class('fr-grid-content')
@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez May 17, 2017

Collaborator

Next steps for the CSS option:

Collaborator

ebruchez commented May 17, 2017

Next steps for the CSS option:

@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez May 17, 2017

Collaborator

Results so far:

  • possible to remove fr-grid-content at runtime and to use a margin instead of padding
  • some controls' CSS need updating, at least I found that the character counter has layout issues this way
  • more work needed for design time (editable grids): Form Builder puts classes on fr-grid-content element, so they have to be moved, and then action-icons.coffee needs updating to put things directly under the fr-grid-td, except fb-hover gets in the way (see #3196).
Collaborator

ebruchez commented May 17, 2017

Results so far:

  • possible to remove fr-grid-content at runtime and to use a margin instead of padding
  • some controls' CSS need updating, at least I found that the character counter has layout issues this way
  • more work needed for design time (editable grids): Form Builder puts classes on fr-grid-content element, so they have to be moved, and then action-icons.coffee needs updating to put things directly under the fr-grid-td, except fb-hover gets in the way (see #3196).
@ebruchez

This comment has been minimized.

Show comment
Hide comment
@ebruchez

ebruchez May 17, 2017

Collaborator

Can we, as a first step, just do the runtime?

Collaborator

ebruchez commented May 17, 2017

Can we, as a first step, just do the runtime?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment