Add separator between columns #18492

faissaloux · 2023-06-08T14:41:54Z

In this PR I have added columns separation which was a bit confusing especially on numbers data.

codecov · 2023-06-08T14:48:00Z

Codecov Report

Patch coverage has no change and project coverage change: -0.06 ⚠️

Comparison is base (4758711) 56.43% compared to head (e9be10a) 56.37%.

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #18492      +/-   ##
============================================
- Coverage     56.43%   56.37%   -0.06%     
- Complexity    16074    16096      +22     
============================================
  Files           635      637       +2     
  Lines         63450    63537      +87     
============================================
+ Hits          35811    35822      +11     
- Misses        27639    27715      +76

Flag	Coverage Δ
dbase-extension	`56.37% <ø> (-0.06%)`	⬇️
recode-extension	`56.34% <ø> (-0.06%)`	⬇️
unit-8.1-ubuntu-latest	`56.34% <ø> (-0.06%)`	⬇️
unit-8.2-ubuntu-latest	`56.34% <ø> (-0.06%)`	⬇️
unit-nightly-ubuntu-latest	`56.30% <ø> (-0.10%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

see 35 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

MauricioFauth · 2023-06-22T19:11:14Z

Is it confusing for RTL languages to have numbers aligned to the end of the cell?
I think the alignment for LTR should not be changed.

faissaloux · 2023-06-24T16:05:41Z

@MauricioFauth The problem is there is no line that separates the columns so the content of a column appears like if it belongs to the next one. Since there is no vertical separator between columns I think when the content is well aligned with the header this makes it obvious which column it belongs. 🤔

MauricioFauth · 2023-06-26T18:58:33Z

@MauricioFauth The problem is there is no line that separates the columns so the content of a column appears like if it belongs to the next one. Since there is no vertical separator between columns I think when the content is well aligned with the header this makes it obvious which column it belongs. 🤔

So the issue is that there's no clear separation between cells, and not the alignment itself.
Aligning text to the start and numbers to the end is the standard for spreadsheet software for a long time, at least for LTR languages.

faissaloux · 2023-06-26T19:17:38Z

So the issue is that there's no clear separation between cells, and not the alignment itself. Aligning text to the start and numbers to the end is the standard for spreadsheet software for a long time, at least for LTR languages.

Yeah sure! I don't have a problem with the numbers in the end of cell, but we need some sort of separation between columns so we don't have that confusion between columns.

Should I update it with columns separators, I think this will solve the issue. What do you think?

MauricioFauth · 2023-06-29T17:18:45Z

Should I update it with columns separators, I think this will solve the issue. What do you think?

Maybe a bit more padding or add a subtle border for the pmahomme theme. But I think this should be done in the master branch, as this is more an enhancement than a bug fix.

phpmyadmin-bot · 2023-07-04T13:31:55Z

This pull requests contains too many commits, in most cases it is caused by wrong merge target. In case you have forked master branch you should also ask merging to master branch. See GitHub documentation for more details.

Signed-off-by: faissaloux <fwahabali@gmail.com>

Fixes phpmyadmin#18471 Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>

Signed-off-by: faissaloux <fwahabali@gmail.com>

faissaloux · 2023-07-04T16:02:47Z

@MauricioFauth I have added separator between column and ready for your review.

williamdes

is there no bootstrap way of doing this?

Signed-off-by: faissaloux <fwahabali@gmail.com>

williamdes · 2023-07-16T08:15:51Z

public/themes/metro/scss/_tables.scss

+  tbody {
+    th,
+    td {
+      border-right: 1px solid #00000017;


Is this change still needed on a bordered table?

MauricioFauth · 2023-07-17T15:53:36Z

templates/display/results/table.twig

@@ -292,7 +292,7 @@
 {% endif %}

  <div class="table-responsive-md">
-    <table class="table table-striped table-hover table-sm table_results data ajax w-auto" data-uniqueId="{{ unique_id }}">
+    <table class="table table-striped table-bordered table-hover table-sm table_results data ajax w-auto" data-uniqueId="{{ unique_id }}">


I'm not convinced that this should be applied to all themes.

@williamdes what do you think?

empty vote, I am not the themes guru here 😄

Sooo! should I revert the last change concerning the table-bordered class 🤔

faissaloux force-pushed the fix-columns-data-alignment branch from aae8ba3 to 89067d0 Compare June 8, 2023 14:42

faissaloux force-pushed the fix-columns-data-alignment branch from 04ada4a to 7dce338 Compare June 8, 2023 15:02

faissaloux changed the base branch from QA_5_2 to master July 4, 2023 13:32

faissaloux force-pushed the fix-columns-data-alignment branch from e81ae14 to 22f0ba2 Compare July 4, 2023 13:35

faissaloux changed the title ~~Fix columns data alignment on LTR and RTL~~ Add separator between columns Jul 4, 2023

faissaloux and others added 9 commits July 4, 2023 15:37

require min php 80102

aeb4b14

Signed-off-by: faissaloux <fwahabali@gmail.com>

Fix SQL statement not being displayed correctly on RTL languages

91d70fc

Fixes phpmyadmin#18471 Signed-off-by: Maurício Meneghini Fauth <mauricio@fauth.dev>

fix columns data alignment

7919081

Signed-off-by: faissaloux <fwahabali@gmail.com>

fix css lint

6c449d2

Signed-off-by: faissaloux <fwahabali@gmail.com>

revert alignment changes

ca6c244

Signed-off-by: faissaloux <fwahabali@gmail.com>

fix columns data alignment

094bda8

Signed-off-by: faissaloux <fwahabali@gmail.com>

fix css lint

3d5b2ed

Signed-off-by: faissaloux <fwahabali@gmail.com>

revert alignment changes

c25e3ef

Signed-off-by: faissaloux <fwahabali@gmail.com>

separate columns by vertical line

58eb79b

Signed-off-by: faissaloux <fwahabali@gmail.com>

faissaloux force-pushed the fix-columns-data-alignment branch from 5f4a07a to 58eb79b Compare July 4, 2023 14:43

faissaloux added 2 commits July 4, 2023 15:48

Merge branch 'master' into fix-columns-data-alignment

c7741ec

fix conflict

89773db

Signed-off-by: faissaloux <fwahabali@gmail.com>

faissaloux force-pushed the fix-columns-data-alignment branch from 334be70 to 89773db Compare July 4, 2023 14:57

williamdes reviewed Jul 13, 2023

View reviewed changes

williamdes added enhancement A feature request for improving phpMyAdmin ui Issues relating to the user interface labels Jul 13, 2023

bootstrap table style

e9be10a

Signed-off-by: faissaloux <fwahabali@gmail.com>

faissaloux force-pushed the fix-columns-data-alignment branch from 5a8a3a6 to e9be10a Compare July 16, 2023 00:56

faissaloux requested a review from williamdes July 16, 2023 00:57

williamdes reviewed Jul 16, 2023

View reviewed changes

MauricioFauth reviewed Jul 17, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add separator between columns #18492

Add separator between columns #18492

faissaloux commented Jun 8, 2023 •

edited

codecov bot commented Jun 8, 2023 •

edited

MauricioFauth commented Jun 22, 2023

faissaloux commented Jun 24, 2023

MauricioFauth commented Jun 26, 2023

faissaloux commented Jun 26, 2023 •

edited

MauricioFauth commented Jun 29, 2023

phpmyadmin-bot commented Jul 4, 2023

faissaloux commented Jul 4, 2023

williamdes left a comment

williamdes Jul 16, 2023

faissaloux Jul 16, 2023

MauricioFauth Jul 17, 2023

faissaloux Jul 17, 2023

williamdes Jul 17, 2023

faissaloux Jul 17, 2023

Add separator between columns #18492

Are you sure you want to change the base?

Add separator between columns #18492

Conversation

faissaloux commented Jun 8, 2023 • edited

codecov bot commented Jun 8, 2023 • edited

Codecov Report

MauricioFauth commented Jun 22, 2023

faissaloux commented Jun 24, 2023

MauricioFauth commented Jun 26, 2023

faissaloux commented Jun 26, 2023 • edited

MauricioFauth commented Jun 29, 2023

phpmyadmin-bot commented Jul 4, 2023

faissaloux commented Jul 4, 2023

williamdes left a comment

Choose a reason for hiding this comment

williamdes Jul 16, 2023

Choose a reason for hiding this comment

faissaloux Jul 16, 2023

Choose a reason for hiding this comment

MauricioFauth Jul 17, 2023

Choose a reason for hiding this comment

faissaloux Jul 17, 2023

Choose a reason for hiding this comment

williamdes Jul 17, 2023

Choose a reason for hiding this comment

faissaloux Jul 17, 2023

Choose a reason for hiding this comment

faissaloux commented Jun 8, 2023 •

edited

codecov bot commented Jun 8, 2023 •

edited

faissaloux commented Jun 26, 2023 •

edited