hmn
/
hmn
4
Fork 9
Code Issues 59 Pull Requests Releases Activity

New project link changed to a bigger button #73

Merged
bvisness merged 3 commits from giggs/hmn:giggs-patch-1 into master 2022-07-26 15:03:36 +00:00
Conversation 3 Commits 3 Files Changed 1 +1 -1
giggs commented 2022-07-24 15:34:41 +00:00
Contributor
Copy Link

Changed the New project link in user profile into a bigger button

preview

Changed the New project link in user profile into a bigger button ![preview](https://i.imgur.com/JHb9hyK.png)
giggs added 1 commit 2022-07-24 15:34:41 +00:00
d52c3350fa Mise à jour de 'src/templates/src/user_profile.html' 
Change the New project link into a button
bvisness requested changes 2022-07-25 17:12:34 +00:00
bvisness left a comment
Owner
Copy Link

Thanks for your contribution! It looks good visually, but the styles are currently in conflict with some other styles in the website. I left some specific comments, but the thrust of it is that we already have a button class for links that should get us most of the way, and padding helpers besides. I'd like to start there to ensure that this button's styles are reasonably consistent with the rest of the site.

Thanks for your contribution! It looks good visually, but the styles are currently in conflict with some other styles in the website. I left some specific comments, but the thrust of it is that we already have a `button` class for links that should get us most of the way, and padding helpers besides. I'd like to start there to ensure that this button's styles are reasonably consistent with the rest of the site.
src/templates/src/user_profile.html Outdated
@ -27,0 +30,4 @@
color: white;
text-align: start;
font-size: 18px;
padding: 15px 26px;
bvisness commented 2022-07-25 17:11:10 +00:00
Owner
Copy Link

We use rem for our spacing and padding instead of px so that the site plays nice with different choices of default font size. We also have lots of utility CSS classes to help us out with this. For example, since I see you want more padding on the sides than on the top and bottom, you could simply put the classes pv2 ph3 on the button. (pv2 = padding vertical 2, ph3 = padding vertical 3.) This helps us use consistent spacing across the site, and makes it easier to do basic layout since we don't have to write a custom CSS class for everything.

Most of our utility classes come from Tachyons, which you can check out here (full list of classes here. We also have a bunch of extra ones defined in core.scss.

We use `rem` for our spacing and padding instead of `px` so that the site plays nice with different choices of default font size. We also have lots of utility CSS classes to help us out with this. For example, since I see you want more padding on the sides than on the top and bottom, you could simply put the classes `pv2 ph3` on the button. (`pv2` = padding vertical 2, `ph3` = padding vertical 3.) This helps us use consistent spacing across the site, and makes it easier to do basic layout since we don't have to write a custom CSS class for everything. Most of our utility classes come from Tachyons, which you can check out [here](https://tachyons.io/) (full list of classes [here](https://tachyons.io/docs/table-of-properties/). We also have a bunch of extra ones defined in core.scss.
src/templates/src/user_profile.html Outdated
@ -27,0 +35,4 @@
cursor: pointer;
}
}
bvisness commented 2022-07-25 17:05:12 +00:00
Owner
Copy Link

This extra closing brace is a syntax error.

This extra closing brace is a syntax error.
src/templates/src/user_profile.html Outdated
@ -136,3 +148,3 @@
{{ if .OwnProfile }}
{{ if .CanAddProject }}
<a href="{{ .NewProjectUrl }}">+ New Project</a>
<<a href="{{ .NewProjectUrl }}" class="button">Add New Project</a>>
bvisness commented 2022-07-25 17:02:54 +00:00
Owner
Copy Link

This looks like broken syntax.

This looks like broken syntax.
bvisness commented 2022-07-25 17:04:42 +00:00
Owner
Copy Link

Also - we actually do already have some styles for links that look like buttons, using the classname button. If you remove your custom CSS for .button, you should already see some styles take effect on this link. How does it look if you keep class="button" but remove your custom CSS?

Also - we actually do already have some styles for links that look like buttons, using the classname `button`. If you remove your custom CSS for `.button`, you should already see some styles take effect on this link. How does it look if you keep `class="button"` but remove your custom CSS?
giggs commented 2022-07-25 19:24:57 +00:00
Author
Contributor
Copy Link

Thanks for your contribution! It looks good visually, but the styles are currently in conflict with some other styles in the website. I left some specific comments, but the thrust of it is that we already have a button class for links that should get us most of the way, and padding helpers besides. I'd like to start there to ensure that this button's styles are reasonably consistent with the rest of the site.

Thanks for the feedback!
I've included a screenshot of what it looks like with the default button class and another with the custom background color + white text.

I personally like the custom style better, especially in dark theme, but it's your call. Let me know which one I should submit for further review

As I mentioned on Discord I'm very new to this, I used an inline style attribute in the link for the custom style. Should I do differently?

> Thanks for your contribution! It looks good visually, but the styles are currently in conflict with some other styles in the website. I left some specific comments, but the thrust of it is that we already have a `button` class for links that should get us most of the way, and padding helpers besides. I'd like to start there to ensure that this button's styles are reasonably consistent with the rest of the site. Thanks for the feedback! I've included a screenshot of what it looks like with the default button class and another with the custom background color + white text. I personally like the custom style better, especially in dark theme, but it's your call. Let me know which one I should submit for further review As I mentioned on Discord I'm *very* new to this, I used an inline style attribute in the link for the custom style. Should I do differently?
default button.PNG
64 KiB
new style.PNG
109 KiB
default button.PNG new style.PNG
giggs added 1 commit 2022-07-25 19:47:59 +00:00
giggs requested review from bvisness 2022-07-25 19:50:44 +00:00
bvisness commented 2022-07-25 20:21:48 +00:00
Owner
Copy Link

I like your new style better too, but I'd also rather not deviate from our typical button styles. What I'd rather do instead is update our button styles globally across the site, and I made a separate issue for that: #74

For now I'd prefer to ship this with just the stock button class, but if you'd like to make a PR exploring alternate styles for the global .button class, that would be very welcome.

I like your new style better too, but I'd also rather not deviate from our typical button styles. What I'd rather do instead is update our button styles globally across the site, and I made a separate issue for that: #74 For now I'd prefer to ship this with just the stock `button` class, but if you'd like to make a PR exploring alternate styles for the global `.button` class, that would be very welcome.
bvisness requested changes 2022-07-25 20:22:38 +00:00
src/templates/src/user_profile.html Outdated
@ -136,3 +136,3 @@
{{ if .OwnProfile }}
{{ if .CanAddProject }}
<a href="{{ .NewProjectUrl }}">+ New Project</a>
<a href="{{ .NewProjectUrl }}" class="button pv2 ph3" style ="background-color: #7e3834; color:white;">Add New Project</a>
bvisness commented 2022-07-25 20:22:32 +00:00
Owner
Copy Link

Indent is weird, but not a huge deal. Let's remove the inline styles per my comment and ship this as is; we can try other styles like this in #74.

Indent is weird, but not a huge deal. Let's remove the inline styles per my comment and ship this as is; we can try other styles like this in #74.
giggs commented 2022-07-25 21:48:07 +00:00
Author
Contributor
Copy Link

Done, thanks!

Done, thanks!
giggs added 1 commit 2022-07-25 21:37:43 +00:00
giggs requested review from bvisness 2022-07-25 21:48:00 +00:00
bvisness approved these changes 2022-07-26 15:03:30 +00:00
bvisness left a comment
Owner
Copy Link

Looks great, thank you!

Looks great, thank you!
bvisness merged commit 12d2fab1b8 into master 2022-07-26 15:03:36 +00:00
bvisness commented 2022-07-26 18:43:55 +00:00
Owner
Copy Link

This is deployed! Looks much better. Thanks again for your contribution!

This is deployed! Looks much better. Thanks again for your contribution!
Sign in to join this conversation.
Reviewers
No reviewers
bvisness
Labels
Clear labels   
admins only

Exclusively for the HMN admin team

  
bug

Something is not working

  
design

Aesthetics required

  
duplicate

This issue or pull request already exists

  
gimme feedback

We would like more community input on this

  
good first issue

   
hmmmm

The HMN team needs to think on this one

  
invalid

Something is wrong

  
reference

Hub for planning and discussion

  
wontfix

This won't be fixed

No Label
admins only
bug
design
duplicate
gimme feedback
good first issue
hmmmm
invalid
reference
wontfix
Milestone
Clear milestone
No items
No Milestone
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: hmn/hmn#73
No description provided.
Delete Branch "giggs/hmn:giggs-patch-1"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?