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

moved sb-admin-2 assets to dependencies #370

Merged
merged 5 commits into from
Oct 14, 2020
Merged

Conversation

marxide
Copy link
Contributor

@marxide marxide commented Oct 3, 2020

Closes #369

@marxide
Copy link
Contributor Author

marxide commented Oct 3, 2020

I think I covered all the changes to the original sb-admin-2 assets. I couldn't do the media query override, but I managed to achieve the same result of a default collapsed sidebar by adding a class in the HTML markup.

@marxide marxide requested review from srggrs and ajstewart October 3, 2020 02:03
@marxide marxide self-assigned this Oct 3, 2020
@marxide marxide added cleanup Clean up code UI User Interface labels Oct 3, 2020
@marxide marxide added this to the Collaboration beta release milestone Oct 3, 2020
Copy link
Contributor

@ajstewart ajstewart left a comment

Choose a reason for hiding this comment

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

Firstly CHANGELOG.md needs updating.

Everything seems to work ok, though the NavBar has changed behaviour, here when I click to 'collapse' it instead shows the full expanded view. It should collapse fully on most screens from the default 'collapsed view'.

We decided to make this collapsed view the default and with the previous sb-admin changes I made it so the expanded NavBar view would only be seen on really high res screens (I think I set it to 3840 pixels).

marxide and others added 2 commits October 6, 2020 16:08
Added a new responsive breakpoint, xxl: 3840px.
Change the sidebar expansion breakpoint to xxl.
@marxide marxide requested a review from ajstewart October 6, 2020 21:12
@marxide
Copy link
Contributor Author

marxide commented Oct 6, 2020

Firstly CHANGELOG.md needs updating.

Thanks. I'll remember to do it the first time one day... 😣

Everything seems to work ok, though the NavBar has changed behaviour, here when I click to 'collapse' it instead shows the full expanded view. It should collapse fully on most screens from the default 'collapsed view'.

We decided to make this collapsed view the default and with the previous sb-admin changes I made it so the expanded NavBar view would only be seen on really high res screens (I think I set it to 3840 pixels).

Yeah, I couldn't figure out how to undo the media query that is declared on the md breakpoint in sb-admin-2. Adding sb-admin-2 as a dependency means we lose the ability to directly modify it as done before since any changes would get overwritten when the package gets updated.

However, I think I've found a way to do it by compiling the sb-admin-2 Sass files ourselves with Gulp. In doing this, I've also changed our own custom pipeline.css to be compiled from the Sass file scss/pipeline.scss. It looks complex, but Gulp should take care of it. If you want to add more CSS rulesets, you can just add them to scss/pipeline.scss, then run Gulp and they'll end up in static/css/pipeline.css. Sass is a superset of CSS, so if you don't have a handle on Sass syntax, you can just write regular CSS too.

ajstewart
ajstewart previously approved these changes Oct 8, 2020
Copy link
Contributor

@ajstewart ajstewart left a comment

Choose a reason for hiding this comment

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

Nice fix! Looks good and seems to all be working well.

ajstewart
ajstewart previously approved these changes Oct 9, 2020
@srggrs srggrs mentioned this pull request Oct 9, 2020
@marxide
Copy link
Contributor Author

marxide commented Oct 14, 2020

Merging is blocked due to a required review but the only thing I changed since the last review was resolving the merge conflict on CHANGELOG.md, so I'm merging this now.

@marxide marxide merged commit 64beea6 into master Oct 14, 2020
@marxide marxide deleted the issue-369-sb-admin-2-dep branch October 14, 2020 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cleanup Clean up code UI User Interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

add sb-admin-2 dependency
3 participants