-
Notifications
You must be signed in to change notification settings - Fork 48
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.
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
Improve standardizations across events and joblistings #3838
base: master
Are you sure you want to change the base?
Improve standardizations across events and joblistings #3838
Conversation
This looks really good already!! 😍 😍 Some comments solely based on the given images:
|
I totally agree!! I'll test it out:) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, this looks very nice. Would love to merge it in sometime soon 😛
border-bottom: 1px solid var(--border-gray); | ||
padding-bottom: 10px; | ||
margin-bottom: 20px; | ||
font-size: 18px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
font-size: 18px; | |
font-size: var(--font-size-lg); |
.picker { | ||
display: flex; | ||
flex-direction: row; | ||
justify-self: flex-end; | ||
align-self: flex-end; | ||
align-items: center; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This div
should instead just be a Flex
component.
.calender { | ||
grid-area: calender; | ||
.title { | ||
font-size: 24px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not a fan of changing the font-size
of headers. Either use the font-sie variables or change the header type, e.g. to h2
which is 24px
.
margin-bottom: 20px; | ||
font-size: 18px; | ||
border-bottom: 2px solid var(--border-gray); | ||
font-weight: 500; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The added font-weight
shouldn't be necessary since .heading
is used on an h2
tag.
grid-area: create; | ||
.header { | ||
display: flex; | ||
justify-content: flex-end; | ||
flex-direction: row; | ||
align-items: center; | ||
gap: 15px; | ||
width: 100%; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please just use the Flex
component for this. Less CSS is better CSS.
background-color: var(--lego-font-color); | ||
border-radius: 100%; | ||
margin-top: 5px; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing new line
activeClassName={styles.active} | ||
className={styles.pickerItem} | ||
> | ||
<Icon name="calendar-clear-outline" size={26}></Icon> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<Icon name="calendar-clear-outline" size={26}></Icon> | |
<Icon name="calendar-clear-outline" size={26} /> |
@@ -2,10 +2,11 @@ | |||
|
|||
.root { | |||
composes: contentContainer from '~app/styles/utilities.css'; | |||
padding: 0px 0px 0px 0px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
padding: 0px 0px 0px 0px; | |
padding: 0; |
@@ -57,6 +57,7 @@ const FilterLink = ({ | |||
value={filters[type].includes(value)} | |||
onChange={handleChange} | |||
readOnly | |||
className={styles.filterCheckbox} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to be used?
@@ -24,11 +24,10 @@ | |||
} | |||
|
|||
.sidebarHeader { | |||
font-size: 1.8em; | |||
font-size: 24px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See comments above regarding font-sizes.
I've been a little inactive on this but I'll try my best of finish this before summer 🤓 |
Description
This is a work-in-progress branch, so I'm not finished yet. There are a lot of small tweaks included to standardize and modernize the design across the webapp's three main sections, respectively "arrangementer", "karriere" and "om abakus".
On all the pages:
I've added a title that is similarly styled across the three pages.
On the karriere-page:
I've refactored the filters into a neat sidebar with a bg-color similar to the updated "om abakus"-page. I've also made some improvements to the margins/paddings as well as upping the thickness of the dividers. Lastly I moved the create new job listing button to the bottom. I'm thinking about replacing this button with a button similar to that on the "arrangementer" page (for consistency).
On the arrangementer-page:
I replaced the NavLinks buttons with icons, as they were not optimal aesthetically speaking and took a lot of space. I've also added a round indicator selector so that you easily can see which view you are using. I'm not completely happy with this design yet, so I'm looking to improve it. I also removed the filter icon :,( since it just looked really messy in-between the elements. Lastly I moved the "opprett nytt" button to be beside the newly created title "arrangementer", for a cleaner look. This is also how I'm thinking about doing the "create new job listing"-button later.
Again this PR is not finished, there is still a lot of improvements to do, but I would appreciate feedback along the way:-) Basicly the goal with this PR is to make everything similar to the new "karriere-page" as I think this one turned out very cleanly.
Result
If you've made visual changes, please include before and after images, preferably with a description. Make sure they do not contain any real user information.
before:
after:
Testing
Resolves ... (either GitHub issue or Linear task)
Closes #ABA-435