[ONBTC] mobile view on onbtc wallet page#6340
[ONBTC] mobile view on onbtc wallet page#6340OscarG673 wants to merge 2 commits intomempool:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR improves the mobile layout of the ONBTC wallet page by making the wallet stats table and addresses treemap more responsive on small screens.
Changes:
- Implements vertical stacking layout for table rows on mobile devices (below 576px)
- Positions the treemap below the table and makes it span full width on mobile (below 768px)
- Adds :host styles to addresses-treemap component for consistent full-width behavior
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| frontend/src/app/components/wallet/wallet.component.scss | Added media queries for mobile table layout (stacked rows) and treemap full-width display |
| frontend/src/app/components/wallet/wallet.component.html | Added semantic class names (addresses-row, addresses-label) to table elements for CSS targeting |
| frontend/src/app/components/addresses-treemap/addresses-treemap.component.scss | Added :host styles and width: 100% to container classes for responsive behavior |
| td { | ||
| display: block; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
The width: 100% rule on td elements may not apply correctly for liquid addresses because the .liquid-address .address-table selector at lines 143-148 has higher specificity and sets fixed widths. Consider increasing the specificity of this mobile selector to ensure it overrides the liquid-address styles, for example by adding .liquid-address to this rule as well or using !important.
| .node-channels-container, | ||
| .addresses-treemap-container { | ||
| position: relative; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
The selector groups .node-channels-container and .addresses-treemap-container together, but these appear to be unrelated components. The node-channels-container is used in a different component (node-channels.component.html). If addresses-treemap.component.html only uses .addresses-treemap-container, consider removing .node-channels-container from this selector to avoid cross-component styling dependencies. Alternatively, if this component file is intentionally shared between both components, this grouping is acceptable but should be documented.
| <tbody> | ||
| <tr> | ||
| <td i18n="address.number-addresses">Addresses</td> | ||
| <tr class="addresses-row"> |
There was a problem hiding this comment.
The class 'addresses-label' is added to the td element but is not used anywhere in the CSS. Consider removing it if it's not needed, or if it's intended for future use, consider adding a comment explaining its purpose.
| <tr class="addresses-row"> | |
| <tr class="addresses-row"> | |
| <!-- 'addresses-label' class is reserved for potential future styling or automated tests --> |
mononaut
left a comment
mononaut
left a comment
There was a problem hiding this comment.
Concept ACK for the treemap fix, which is long overdue.
but the table layout intended to remain as two columns on mobile in order to match the many other pages with similar tables. if we do decide to redesign this, we should apply the new layout everywhere, but I think best to leave that for a future PR for now.
thank you for your feedback, @mononaut. I reverted the changes made to the table, I consider that at least on mobile it would work better, perhaps in the future it could be implemented, so i'm just leaving it as a suggestion :) I also noticed that the treemap had a tooltip that was not constrained to the component. I've added let me know, thanks! new.mov |
| width: 100%; | ||
| } | ||
|
|
||
| .node-channels-container, |
There was a problem hiding this comment.
| .node-channels-container, |
| :host { | ||
| display: block; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
this applies the same styles to the same element as the
@media (max-width: 767.98px) {
.treemap-col app-addresses-treemap {
...
}
}
in wallet.component.scss, except without the @media (max-width: 767.98px) limitation.
decide if you need these rules to apply everywhere, or just in the wallet context on mobile, or not at all, and remove one or both accordingly.
| .treemap-col { | ||
| width: 45%; | ||
| height: 300px; | ||
|
|
||
| @media (max-width: 767.98px) { | ||
| width: 100%; | ||
| min-width: 0; | ||
| } | ||
| } |
There was a problem hiding this comment.
it would probably be cleaner to invert the media rule here, since I think the default col-md style is already width: 100%.
or there might be a smarter way to do this directly in bootstrap classes 🤔
Summary
improves the mobile layout of the wallet stats table and addresses treemap by adjusting their structure for better responsiveness and readability on small screens
Result
on mobile devices:
table rows are displayed in a stacked (vertical) layout instead of side by side
the addresses treemap is positioned below the table
the treemap now spans the full screen width
screenshot (iphone 12 pro):
fixes: #6339