Skip to content

Conversation

@aybaof
Copy link

@aybaof aybaof commented Nov 27, 2022

No description provided.

@aybaof aybaof force-pushed the git_revert_index_change branch 2 times, most recently from e4284be to 45d92d1 Compare November 27, 2022 10:28
fixes PR to squash

prettier to squash

config prettier vscode != cli
@aybaof aybaof force-pushed the git_revert_index_change branch from 45d92d1 to 39c7286 Compare November 27, 2022 10:43
@aybaof
Copy link
Author

aybaof commented Nov 27, 2022

Fixes last pull request with integration of bootstrap / font correctly and rework of sidemenu using flex instead of table

@dannycho7 dannycho7 self-requested a review November 27, 2022 15:43
@dannycho7
Copy link
Contributor

Could you remind me what the PR is doing and also post screenshots of the intended UI after this change?

Screen Shot 2022-11-28 at 11 02 29 PM

This is what I see on my end, but I'm not entirely sure all the changes are expected.

Comment on lines +1 to +9
/* #switch_week_button {
background-color: #2cb8f0;
color: white;
padding: 15px;
width: 15%;
height: 2%;
text-decoration: none;
text-align: center;
/* font-family: 'Billionary';*/
vertical-align: middle;
font-size: 2rem;
}

.next-week-link {
background-color: #2cb8f0;
position: fixed;
right: 1.5%;
bottom: 0;
}

.this-week-link {
background-color: #e844f4;
position: fixed;
left: 0;
bottom: 0;
}
min-width: 300px;
} */
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we get rid of any comments of code? Comments are of course fine if they are describing some piece of code, but code that's commented out seem to not add any value and take up unnecessary space/complexity.

Copy link
Author

Choose a reason for hiding this comment

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

Yes sorry its mainly left overs of old code.
Would it be ok to merge it for now , and i will make a commit afterwards removing all unnecessary comment in css later ?

Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like the comment was added in this PR, so it would be good to remove it here.

@aybaof
Copy link
Author

aybaof commented Nov 29, 2022

This PR is mainly about start including bootstrap theme on index, and rework some layout like the sidenav.
UI gonna have a rework so there is no to little change on this part.

@dannycho7
Copy link
Contributor

This PR is mainly about start including bootstrap theme on index, and rework some layout like the sidenav. UI gonna have a rework so there is no to little change on this part.

It looks like there are changes to the UI from this PR though. Could you take a look at the screenshot I sent and lmk if that is intended? For example, the "next week" text on the top left is noticeably smaller.

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