Skip to content
Jump to bottom

Automated Test: header-layout-optimization-post #329

Closed
wants to merge 1 commit into from
+54 −13
Closed

Automated Test: header-layout-optimization-post #329

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
9 changes: 5 additions & 4 deletions app/assets/stylesheets/common/base/header.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,9 @@

.contents {
margin: 8px 0;
}

.title {
float: left;
@include flexbox();
@include align-items(center);
}

.valign-helper {
Expand All @@ -34,8 +33,10 @@
}

.panel {
float: right;
position: relative;
margin-left: auto;

@include order(3);
}

.login-button, button.sign-up-button {
Expand Down
9 changes: 5 additions & 4 deletions app/assets/stylesheets/common/base/topic-post.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,9 @@ table.md-table {
}

.small-action {
@include flexbox();
@include align-items(center);

Comment on lines 263 to +266
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟠 Major

Flexbox on .small-action looks good, but float on child elements is now a no-op.

With .small-action as a flex container, float: left on .topic-avatar (line 270) and float: right on button (line 309) are both ignored per the CSS spec. The avatar still appears on the left naturally (flex items flow left-to-right), but the button will no longer be right-aligned since float: right has no effect inside a flex container.

To restore the right-alignment, add margin-left: auto to the button rule (or to .small-action-desc if you want the button pushed to the far right).

Proposed fix 
   button {
     background: transparent;
     border: 0;
-    float: right;
+    margin-left: auto;
   }
🤖 Prompt for AI Agents 
In `@app/assets/stylesheets/common/base/topic-post.scss` around lines 263 - 266,
The flex container .small-action makes child floats no-op, so update the button
alignment by adding margin-left: auto to the button rule (or to
.small-action-desc to push the button to the far right); also remove or keep the
float on .topic-avatar as optional (avatar naturally sits left in flex) and
ensure .topic-avatar and button selectors are adjusted accordingly so the button
is right-aligned inside .small-action.

.topic-avatar {
padding: 5px 0;
border-top: none;
Expand All @@ -274,8 +277,7 @@ table.md-table {
}

.small-action-desc {
padding: 0.5em 0 0.5em 4em;
margin-top: 5px;
padding: 0 1.5%;
text-transform: uppercase;
font-weight: bold;
font-size: 0.9em;
Expand All @@ -287,7 +289,7 @@ table.md-table {
font-weight: normal;
font-size: 14px;
p {
margin: 5px 0;
margin: 0;
}
}

Expand All @@ -298,7 +300,6 @@ table.md-table {

> p {
margin: 0;
padding-top: 4px;
}
}

Expand Down
6 changes: 3 additions & 3 deletions app/assets/stylesheets/common/base/topic.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,11 +27,11 @@
}

.extra-info-wrapper {
@include order(2);

line-height: 1.5;
.badge-wrapper {
float: left;
&.bullet {
margin-top: 5px;
}
}

}
Expand Down
6 changes: 4 additions & 2 deletions app/assets/stylesheets/common/components/badges.css.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,10 @@


&.bullet { //bullet category style
display: inline-flex;
align-items: baseline;
@include inline-flex();

@include align-items(baseline);

margin-right: 10px;

span.badge-category {
Expand Down
37 changes: 37 additions & 0 deletions app/assets/stylesheets/common/foundation/mixins.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,40 @@
-moz-user-select: none;
-ms-user-select: none;
}

// ---------------------------------------------------

//Flexbox

@mixin flexbox() {
display: -webkit-box;
display: -moz-box;
display: -ms-flexbox;
display: -webkit-flex;
display: flex;
}

@mixin inline-flex() {
display: -webkit-inline-box;
display: -webkit-inline-flex;
display: -moz-inline-box;
display: -ms-inline-flexbox;
display: inline-flex;
}


@mixin align-items($alignment) {
-webkit-box-align: $alignment;
-webkit-align-items: $alignment;
-ms-flex-align: $alignment;
-ms-align-items: $alignment;
align-items:$alignment;
}
Comment on lines +117 to +123
Copy link

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor

-ms-align-items on line 121 is not a valid CSS property.

The IE10 flexbox property is -ms-flex-align, which is already declared on line 120. Line 121 (-ms-align-items) is a no-op in every browser and should be removed.

Also, note that -webkit-box-align and -ms-flex-align use a different value vocabulary than standard align-items (e.g., start/end vs flex-start/flex-end). The current call sites pass center and baseline, which happen to be shared values, but passing flex-start or flex-end would silently fail in legacy browsers.

Proposed fix 
 `@mixin` align-items($alignment) {
     -webkit-box-align: $alignment;
     -webkit-align-items: $alignment;
-    -ms-flex-align: $alignment;  
-    -ms-align-items: $alignment;
+    -ms-flex-align: $alignment;
     align-items:$alignment;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
@mixin align-items($alignment) {
-webkit-box-align: $alignment;
-webkit-align-items: $alignment;
-ms-flex-align: $alignment;
-ms-align-items: $alignment;
align-items:$alignment;
}
`@mixin` align-items($alignment) {
-webkit-box-align: $alignment;
-webkit-align-items: $alignment;
-ms-flex-align: $alignment;
align-items:$alignment;
}
🤖 Prompt for AI Agents 
In `@app/assets/stylesheets/common/foundation/mixins.scss` around lines 117 - 123,
Remove the invalid property declaration `-ms-align-items` from the mixin and
update the mixin align-items($alignment) to normalize modern align-items values
for legacy prefixed properties: map "flex-start" -> "start" and "flex-end" ->
"end" (leave "center" and "baseline" unchanged), use the mapped value for both
`-webkit-box-align` and `-ms-flex-align`, keep `-webkit-align-items` and
standard `align-items` using the original $alignment, and ensure only
`-ms-flex-align` (not `-ms-align-items`) is emitted for IE10 support.


@mixin order($val) {
-webkit-box-ordinal-group: $val;
-moz-box-ordinal-group: $val;
-ms-flex-order: $val;
-webkit-order: $val;
order: $val;
}