Merge pull request opf#17279 from opf/bug/59343-spacing-between-table…

…-columns-is-off-if-name-is-long

[#59343] Spacing between table columns is off if name is long

app/components/op_primer/border_box_row_component.rb

@@ -45,7 +45,8 @@ def visible_on_mobile?(column)
     def grid_column_classes(column)
       classes = ["op-border-box-grid--row-item"]
       classes << column_css_class(column)
-      classes << "op-border-box-grid--wide-column" if table.wide_column?(column)
+      classes << "op-border-box-grid--main-column" if table.main_column?(column)
+      classes << "ellipsis" unless table.main_column?(column)
       classes << "op-border-box-grid--no-mobile" unless visible_on_mobile?(column)
 
       classes.compact.join(" ")

app/components/op_primer/border_box_table_component.html.erb

@@ -34,14 +34,15 @@ See COPYRIGHT and LICENSE files for more details.
       test_selector:,
     )) do |component|
     component.with_header(classes: grid_class, color: :muted) do
-      concat render(Primer::Beta::Text.new(classes: "op-border-box-grid--mobile-heading",
-                                           font_weight: :semibold)) { mobile_title }
       headers.each do |name, args|
         concat render(Primer::Beta::Text.new(classes: header_classes(name),
                                              font_weight: :semibold,
                                              **header_args(name))) { args[:caption] }
       end
 
+      concat render(Primer::Beta::Text.new(classes: "op-border-box-grid--mobile-heading",
+                                           font_weight: :semibold)) { mobile_title }
+
       if has_actions?
         concat render(Primer::BaseComponent.new(classes: heading_class,
                                                 tag: :div))

app/components/op_primer/border_box_table_component.rb

@@ -59,22 +59,22 @@ def mobile_labels(*names)
         @mobile_labels = names.map(&:to_sym)
       end
 
-      # Declare wide columns, that will result in a grid column span of 3
+      # Declare main columns, that will result in a grid column span of 2 and not truncate text
       #
       #     column_grid_span :title
       #
-      def wide_columns(*names)
-        return Array(@wide_columns) if names.empty?
+      def main_column(*names)
+        return Array(@main_columns) if names.empty?
 
-        @wide_columns = names.map(&:to_sym)
+        @main_columns = names.map(&:to_sym)
       end
     end
 
     delegate :mobile_columns, :mobile_labels,
              to: :class
 
-    def wide_column?(column)
-      self.class.wide_columns.include?(column)
+    def main_column?(column)
+      self.class.main_column.include?(column)
     end
 
     def header_args(_column)
@@ -88,7 +88,7 @@ def column_title(name)
 
     def header_classes(column)
       classes = [heading_class]
-      classes << "op-border-box-grid--wide-column" if wide_column?(column)
+      classes << "op-border-box-grid--main-column" if main_column?(column)
 
       classes.join(" ")
     end

app/components/op_primer/border_box_table_component.sass

@@ -15,12 +15,20 @@
     justify-content: flex-start
     align-items: center
 
+    &--heading,
+    &--row-item
+      &:not(:first-child)
+        padding-left: 6px
+
+      &:not(:last-child)
+        padding-right: 6px
+
     &--mobile-heading,
     &--row-label
       display: none
 
-    &--wide-column
-      grid-column: span 3
+    &--main-column
+      grid-column: span 2
 
 @media screen and (max-width: $breakpoint-md)
   .op-border-box-grid

lookbook/docs/patterns/12-tables.md.erb

@@ -35,7 +35,7 @@ end
 
 
 
-### Border Box Table specific options
+### Border Box Table specifics
 
 #### Mobile behavior
 
@@ -60,18 +60,22 @@ mobile_labels :users
 
 On mobile, the usual table headers are replaced with a single `mobile_title` property that you have to set on the table.
 
-#### Wide columns
+#### Text wrapping behaviour
 
-To use wider columns on desktop, use the `wide_columns`  helper:
+By default, text longer than the column width will truncate with ellipsis. Only the main column has text that wraps around to display the full string.
+
+#### Main column
+
+Use the `main_column` helper to make a column 2 times as wide as the others and also display the full text:
 
 ```ruby
 # On desktop, show these columns
 columns :title, :users, :created_at
 
-# Make title column wider than the others (factor 3 using grid span)
-wide_columns :title
+# Make title column wider than the others (factor 2 using grid span)
+main_column :title
 ```
-
+Note: Ideally, one one main column will be present for each table.
 
 
 

lookbook/previews/op_primer/border_box_table_component_preview/custom_column_widths.html.erb

@@ -2,7 +2,7 @@
   table = Class.new(OpPrimer::BorderBoxTableComponent) do
     columns :foo, :bar
 
-    wide_columns :foo
+    main_column :foo
 
     def self.name
       "MyTable"

modules/auth_saml/app/components/saml/providers/table_component.rb

@@ -7,7 +7,7 @@ class TableComponent < ::OpPrimer::BorderBoxTableComponent
 
       mobile_labels :users
 
-      wide_columns :name
+      main_column :name
 
       def initial_sort
         %i[id asc]

modules/meeting/app/components/meetings/table_component.rb

@@ -38,6 +38,8 @@ class TableComponent < ::OpPrimer::BorderBoxTableComponent
 
     mobile_labels :project_name
 
+    main_column :title
+
     def sortable?
       true
     end

modules/openid_connect/app/components/openid_connect/providers/table_component.rb

@@ -3,7 +3,7 @@ module Providers
     class TableComponent < ::OpPrimer::BorderBoxTableComponent
       columns :name, :type, :users, :creator, :created_at
 
-      wide_columns :name
+      main_column :name
 
       mobile_columns :name, :type, :users