Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improved deck recap component layout #111

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Improved deck recap component layout #111

Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
722b4a6
Improved deck recap component layout
piedpiper358 Aug 11, 2022
9237621
Free up more space for deck name
piedpiper358 Aug 30, 2022
850078f
Merge branch 'master' into deck-recap
piedpiper358 Aug 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 7 additions & 9 deletions core/src/css/component/decktracker/main/decktracker-deck-recap.component.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
max-width: 100%;

.title {
color: var(--secondary-text-color);
color: var(--color-1);
margin-bottom: 30px;
}

Expand All @@ -29,7 +29,7 @@
width: 75px;
position: relative;
flex-grow: 0;
margin-right: 20px;
margin-right: 15px;
flex-shrink: 0;

.skin {
Expand All @@ -48,13 +48,14 @@
display: flex;
flex-direction: column;
justify-content: flex-start;
width: 150%;
Copy link
Contributor

Choose a reason for hiding this comment

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

Why this 150%?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to make the deck title block wider. Without it, it would look like this.

2022-09-06_12-53-56

Copy link
Contributor

Choose a reason for hiding this comment

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

I think I understand what the issue is:

  • The "deck-summary" element takes the full width
  • It's divided between deck-image, deck-title and best-against
  • deck-image has a fixed width, best-against is set to take 100% of the space

This means that the layout itself has to figure out what's the best way to give room to each widget.
There are multiple solutions:

  • Attribute width to each element so that the total is 100% (you can have a mix of pixel width + flex-grow: 1)
  • Keep the sizes dynamic, using flex-basis to tell the layout the relative size of the items

In this specific case, the component's width will always be constrained by the width of the ads column, so 400px. So I'm fine with using absolute pixel sizes in this case, but the other option is fine too.

padding-right: 2%;

.deck-name {
color: var(--default-title-color);
margin-bottom: 15px;
display: flex;
flex-direction: column;
align-items: center;

.archetype {
display: flex;
Expand All @@ -72,8 +73,6 @@
--icon-color: var(--default-text-color);
align-items: center;
flex-grow: 0;
justify-content: flex-end;
margin-left: auto;

.watch {
margin-left: 10px;
Expand Down Expand Up @@ -103,11 +102,12 @@
}

.best-against {
margin-left: 40px;
border-left: 1px solid var(--default-text-color);
padding-left: 40px;
width: 100%;
display: flex;
flex-direction: column;
align-items: center;
padding-left: 3%;

.header {
color: var(--default-title-color);
Expand All @@ -118,8 +118,6 @@

.classes {
display: flex;
position: relative;
left: -8px;

.class-icon {
width: 30px;
Expand Down