Skip to content

refactor(proposal): minor UI improvement#2131

Merged
Junjiequan merged 3 commits intomasterfrom
proposal-page-improve
Dec 3, 2025
Merged

refactor(proposal): minor UI improvement#2131
Junjiequan merged 3 commits intomasterfrom
proposal-page-improve

Conversation

@Junjiequan
Copy link
Copy Markdown
Member

@Junjiequan Junjiequan commented Dec 3, 2025

Description

This PR includes following changes:

  • enabled sideFilterCollapsed flag for all components
  • fixed pagination fields disappearing on mobile view
  • stretched the search box to 100% width on mobile
  • truncketed table headers with ellipsis instead of wrapping into two rows

screenshots

image image

Motivation

Background on use case, changes needed

Fixes:

Please provide a list of the fixes implemented in this PR

  • Items added

Changes:

Please provide a list of the changes implemented by this PR

  • changes made

Tests included

  • Included for each change/fix?
  • Passing? (Merge will not be approved unless this is checked)

Documentation

  • swagger documentation updated [required]
  • official documentation updated [nice-to-have]

official documentation info

If you have updated the official documentation, please provide PR # and URL of the pages where the updates are included

Backend version

  • Does it require a specific version of the backend
  • which version of the backend is required:

Summary by Sourcery

Apply truncation with ellipsis to dynamic table header text and adjust table header layout and filter UI responsiveness.

Bug Fixes:

  • Prevent dynamic table header cell text from overflowing by truncating with ellipsis.

Enhancements:

  • Make dynamic table header controls layout more responsive, including wrapping and updated max-width behavior for the global search.
  • Simplify the condition for applying the side-filter header style to depend only on the collapsed state.
  • Update the shared filter facet search label text to clarify its purpose.

@Junjiequan Junjiequan requested a review from a team as a code owner December 3, 2025 12:58
Copy link
Copy Markdown
Contributor

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey there - I've reviewed your changes - here's some feedback:

  • The change to [class.with-side-filter] removes the localization === 'proposal' constraint; please double‑check that this behavior is intended for all localizations and won't affect layouts where the side filter should not shift the header.
  • Using ::ng-deep for .mat-mdc-header-cell .mat-sort-header-content may cause future maintenance issues since it's deprecated; consider scoping this style via a custom class on the table/header cell or moving it to a global stylesheet instead.
Prompt for AI Agents
Please address the comments from this code review:

## Overall Comments
- The change to `[class.with-side-filter]` removes the `localization === 'proposal'` constraint; please double‑check that this behavior is intended for all localizations and won't affect layouts where the side filter should not shift the header.
- Using `::ng-deep` for `.mat-mdc-header-cell .mat-sort-header-content` may cause future maintenance issues since it's deprecated; consider scoping this style via a custom class on the table/header cell or moving it to a global stylesheet instead.

## Individual Comments

### Comment 1
<location> `src/app/shared/modules/dynamic-material-table/table/dynamic-mat-table.component.scss:353` </location>
<code_context>
   display: flex;
   align-items: center;
+  flex-wrap: wrap;
+  justify-content: space-between;
   gap: 1rem;
   margin: 1rem 0;
-  flex-wrap: nowrap;

   &.with-side-filter {
</code_context>

<issue_to_address>
**suggestion:** Using space-between with wrapping may create uneven spacing between header controls on different screen widths.

With `flex-wrap: wrap` and `justify-content: space-between`, the first row can end up with large gaps while wrapped items on subsequent lines stay left-aligned, causing an unbalanced layout at some widths. Consider using `justify-content: flex-start` with `gap`, or restricting which children can flex (e.g. only `.global-search-wrapper` grows, others use `flex: 0 0 auto`). Please verify the layout at intermediate widths, especially near the new 1340px breakpoint.
</issue_to_address>

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

@Junjiequan Junjiequan changed the title apply ellipsis to table header text refactor(proposal): minor UI improvement Dec 3, 2025
@Junjiequan Junjiequan changed the title refactor(proposal): minor UI improvement refactor(proposal): minor UI improvement Dec 3, 2025
@Junjiequan Junjiequan changed the title refactor(proposal): minor UI improvement refactor(proposal): minor UI improvement Dec 3, 2025
@Junjiequan Junjiequan force-pushed the proposal-page-improve branch from fa56a60 to 9393033 Compare December 3, 2025 14:43
@Junjiequan Junjiequan force-pushed the proposal-page-improve branch from 9393033 to c3757ec Compare December 3, 2025 14:51
@Junjiequan Junjiequan enabled auto-merge (squash) December 3, 2025 14:54
@Junjiequan Junjiequan merged commit ee7e108 into master Dec 3, 2025
7 checks passed
@Junjiequan Junjiequan deleted the proposal-page-improve branch December 3, 2025 15:00
Junjiequan added a commit that referenced this pull request Dec 4, 2025
## Description
This PR includes following changes:
- enabled sideFilterCollapsed flag for all components 
- fixed pagination fields disappearing on mobile view
- stretched the search box to 100% width on mobile
- truncketed table headers with ellipsis instead of wrapping into two
rows

### screenshots

<img width="345" height="258" alt="image"
src="https://github.com/user-attachments/assets/a14b3e2c-a68f-4153-b552-83fdbae1f27e"
/>

<img width="278" height="112" alt="image"
src="https://github.com/user-attachments/assets/7767aead-7b5d-46f4-bd3e-ce0026c21f99"
/>


## Motivation
Background on use case, changes needed


## Fixes:
Please provide a list of the fixes implemented in this PR

* Items added


## Changes:
Please provide a list of the changes implemented by this PR

* changes made


## Tests included
- [ ] Included for each change/fix?
- [ ] Passing? (Merge will not be approved unless this is checked) 

## Documentation
- [ ] swagger documentation updated \[required\]
- [ ] official documentation updated \[nice-to-have\]

### official documentation info
If you have updated the official documentation, please provide PR # and
URL of the pages where the updates are included

## Backend version
- [ ] Does it require a specific version of the backend
- which version of the backend is required:

## Summary by Sourcery

Apply truncation with ellipsis to dynamic table header text and adjust
table header layout and filter UI responsiveness.

Bug Fixes:
- Prevent dynamic table header cell text from overflowing by truncating
with ellipsis.

Enhancements:
- Make dynamic table header controls layout more responsive, including
wrapping and updated max-width behavior for the global search.
- Simplify the condition for applying the side-filter header style to
depend only on the collapsed state.
- Update the shared filter facet search label text to clarify its
purpose.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants