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

Install the styleguide module #3

Closed
4 of 8 tasks
clarkepaul opened this issue Jul 2, 2019 · 16 comments
Closed
4 of 8 tasks

Install the styleguide module #3

clarkepaul opened this issue Jul 2, 2019 · 16 comments
Assignees

Comments

@clarkepaul
Copy link
Contributor

clarkepaul commented Jul 2, 2019

As we don't have a good view of all the produced styles of the theme, installing this module might be able to help us identify which styles are not working properly.

My alternative to the style guide would be to create a test page with raw bootstrap example code to see how it generates. If the style guide is fairly automatic (?) then that would be my preference.
cc @chillu

Overview

  • As a member of the PUX team, I want to be able to quickly glance at most styles in the Bambusa theme to identify possible shortcomings
  • As a content editor, I want to know the bambusa theme correctly styles a variety of elements when writing my content.

Acceptance Criteria

  • A "style guide" page is automatically generated by the bambusa site
  • All Common elements are displayed in the style guide
  • The page can be reach via the site navigation.
  • The styleguide only shows elements that are included in the theme (and don't look broken)

Notes

Pull requests

@maxime-rainville
Copy link

I believed we're talking about this module. https://github.com/benmanu/silverstripe-styleguide

@maxime-rainville
Copy link

Made up some ACs and added an overview

@chillu chillu added this to the Bambusa 1.0 milestone Jul 8, 2019
@maxime-rainville maxime-rainville self-assigned this Jul 9, 2019
@maxime-rainville
Copy link

https://github.com/benmanu/silverstripe-simple-styleguide looks like it's more recent and better supported.

@maxime-rainville
Copy link

Done. I've moved to peer review. We'll need to add a menu entry for it, because the guide is on its own controller.

Couple extra point we might want to consider:

  • the module support a colour section that list a predefined list of colour, we could hook our dynamic colour in there. I've disabled it for now.
  • There a TOC at the top of the page, I would want to move that to the side bar.

@chillu chillu assigned bergice and unassigned maxime-rainville Jul 9, 2019
@chillu
Copy link
Member

chillu commented Jul 9, 2019

we could hook our dynamic colour in there

Yeah, nah.

@bergice bergice removed their assignment Jul 11, 2019
@bergice
Copy link
Contributor

bergice commented Jul 11, 2019

Moving to content for the menu entry to be added.

@chillu
Copy link
Member

chillu commented Jul 11, 2019

@bergice Looks OK, but since the styling wasn't shown in the actual container of the theme, it looked quite off - no left padding, and some styles only apply within that container. I've spent five minutes to fix this, can you please review? silverstripe/bambusa-installer#25

@chillu
Copy link
Member

chillu commented Jul 11, 2019

@clarkepaul @Karlstir Probably not super obvious unless you hit the module README - the styleguide is accessible through the /_styleguide URL, which you can link from other places - it won't be "discoverable" unless it's explicitly linked.

I've added an intro to make the intent of this page a bit clearer:

This view is only accessible to developers and admin users. It demonstrates styling variations, and can be used to test styling changes.

@maxime-rainville @bergice You've both missed that there's a isDev() check in the controller, so this styleguide won't fulfill it's purpose - it's not available on a demo which runs in isLive() mode. Can you please submit a PR to the module to make this check configurable, and opt out of it for Bambusa? Keep the ADMIN check in place.

@bergice
Copy link
Contributor

bergice commented Jul 11, 2019

@bergice Looks OK, but since the styling wasn't shown in the actual container of the theme, it looked quite off - no left padding, and some styles only apply within that container. I've spent five minutes to fix this, can you please review? silverstripe/bambusa-installer#25

Looks good. I'm guessing you want this merged in bambusa-theme instead of bambusa-installer?

@bergice
Copy link
Contributor

bergice commented Jul 11, 2019

@clarkepaul @Karlstir Probably not super obvious unless you hit the module README - the styleguide is accessible through the /_styleguide URL, which you can link from other places - it won't be "discoverable" unless it's explicitly linked.

I've added an intro to make the intent of this page a bit clearer:

This view is only accessible to developers and admin users. It demonstrates styling variations, and can be used to test styling changes.

@maxime-rainville @bergice You've both missed that there's a isDev() check in the controller, so this styleguide won't fulfill it's purpose - it's not available on a demo which runs in isLive() mode. Can you please submit a PR to the module to make this check configurable, and opt out of it for Bambusa? Keep the ADMIN check in place.

That's added to the PR's.

@chillu
Copy link
Member

chillu commented Jul 11, 2019

Looks good. I'm guessing you want this merged in bambusa-theme instead of bambusa-installer?

No, since the theme doesn't require the style guide, the "app" does (bambusa-installer) - so it's in the right place.

@bergice bergice removed their assignment Jul 11, 2019
@chillu
Copy link
Member

chillu commented Jul 11, 2019

benmanu/silverstripe-simple-styleguide#12 add dev toggle

That's a blocker now, unless we run a fork. The module isn't operational in the way we need it to be without this in place, right?

@maxime-rainville
Copy link

We could just extend that class and run our own index method with a more permissive check.

@maxime-rainville
Copy link

I'm moving this to block since we're waiting for the module maintainer to action our pull requests.

If we get to the end of the project and they still haven't approved our PRs we can look at ways of bypassing those issues.

@chillu
Copy link
Member

chillu commented Jul 25, 2019

@silverstripeux This is on UAT now: http://bambusad-uat.sites.silverstripe.com/styleguide/. Had a quick check and the styleguide looks OK to me, but you might be able to spot more issues with your eagle design eyes 🦅

@chillu chillu removed their assignment Jul 25, 2019
@clarkepaul
Copy link
Contributor Author

Installation of the module is fine and I've added some links to it from various places with the content:

There are still styling issues still with spacing and image alignment but they are outside of this issue.

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

No branches or pull requests

4 participants