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.

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

Made onsite buttons #3252

Merged
merged 6 commits into from
May 7, 2021
Merged

Made onsite buttons #3252

merged 6 commits into from
May 7, 2021

Conversation

nrjean
Copy link
Contributor

@nrjean nrjean commented Apr 9, 2021

Makes onsite links into buttons - also introduces a class called makeButtonLink which can be called to make a link that is a button.
Should be useful if anyone is making a new page and wants to make a link, that is you know, a button.

If anyone has any comments on how I could make the onsite page prettyier that would be great.

@nrjean
Copy link
Contributor Author

nrjean commented Apr 9, 2021

Fixes #3025 sorta

@hwatheod
Copy link
Contributor

There are 2 new functions called makeButtonLink but it's only called once in the template - which one is being called and what is the other one for? (Or somehow both are being called depending on module ?)

@nrjean
Copy link
Contributor Author

nrjean commented Apr 11, 2021

So the first function in modules/base.py is a global function that works just like makeLink but when it makes the link it makes it a link that a button. It makes just one link per module.

In handlers/onsiteclasslist.py multiple links need to be generated for that module so is has an alternate function which essentially calls makeButtonLink twice for each link that is needed.

@willgearty can probably explain the technical of this better lol

{% else %}
<li><em>{{ module.module.link_title }} (below)</em></li>
<<em>{{ module.module.link_title }} (below)</em><br>
Copy link
Contributor

Choose a reason for hiding this comment

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

There shouldn't be two < signs here

@hwatheod
Copy link
Contributor

So the first function in modules/base.py is a global function that works just like makeLink but when it makes the link it makes it a link that a button. It makes just one link per module.

In handlers/onsiteclasslist.py multiple links need to be generated for that module so is has an alternate function which essentially calls makeButtonLink twice for each link that is needed.

Oh I see, the first one is used for most of the onsite modules, and the second one in a subclass which overrides the first, and is used for one particular special module. Got it.

@willgearty
Copy link
Member

willgearty commented Apr 16, 2021

Code looks good (aside from the comments from @hwatheod). However, the page currently looks like this:
image

This doesn't seem much better than the links from before, in my opinion. But, I think if you include the "management.css" file you'll be much closer, since it should make the buttons look somewhat like the ones on the main management page:
image

But you'll probably still want to play around with it to make sure they have the same heights, etc.

I'd also suggest reducing the space at the top of the template (looks like there are four line breaks for some reason?)

@nrjean
Copy link
Contributor Author

nrjean commented May 7, 2021

fixed a teds changes and also make it look better by organizing the buttons yay @willgearty

Copy link
Member

@willgearty willgearty left a comment

Choose a reason for hiding this comment

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

Code looks good to me, and it works well on my dev server! What a difference it makes!

@willgearty willgearty merged commit d53fa4c into main May 7, 2021
@willgearty willgearty deleted the make_onsite_pretty_again branch May 7, 2021 13:57
willgearty added a commit that referenced this pull request May 8, 2021
willgearty added a commit that referenced this pull request May 27, 2021
* Initial docs for stable release 13

* Docs for #3116, #3117, and #3118

* Added docs about django upgrade

* Docs for #3128

* Docs for #3129, #3133, #3134, and #3137

* Docs for #3156 and #3153

* Docs for #3174, #3163, and #3184

* Docs for #3139, #3140, and #3141

* Docs for #3143, #3150, #3154, #3160, #3162, and #3168

* Docs for #3171, #3185, #3186, and #3188

* Docs for #3131 and #3189

* Docs for #3149 and #3190

* Docs for #3193, #3194, #3195, #196, and #3197

* Clarification

* Docs for #3192, #3201, #3209, and #2248

* Docs for #3204, #3212, #3214, #3205, 9fd073c, and #3226

* Docs for #3232, de5861c, #3231, #3233, #3234, #3237, #3238, and #3239

* Fix indent

* Docs for #3227 and #3235

* Add missing word

* spelling

* Docs for e57581f, #3255, #3253, #3257, and #3249

* Docs for #3254, #3260, and #3262

* Docs for #3263, #3272, #3240, #3264, #3266, and #3270

* clarifications

* Docs for #3283 and #3252

* Docs for #3288 and misc commits

* Docs for #3292, #3311, #3286, #3289, and #3279

* Docs for a377f0d; move note

* Docs for #3315, #3290, and #3322

* Docs for #3273 and #3317

* Final edits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants