-
Notifications
You must be signed in to change notification settings - Fork 25
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
Add a Bootstrap layout to the CRUD add-on #485
Add a Bootstrap layout to the CRUD add-on #485
Conversation
de259a3
to
9e57e90
Compare
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 file will be reviewed and improved in #491
53a5378
to
d15961a
Compare
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.
I have some comments and small suggestions 🙏🏻
.template/addons/crud/app/assets/stylesheets/layouts/_application.scss
Outdated
Show resolved
Hide resolved
.template/addons/crud/app/assets/stylesheets/layouts/_application.scss
Outdated
Show resolved
Hide resolved
.template/addons/crud/app/assets/stylesheets/layouts/index.scss
Outdated
Show resolved
Hide resolved
.template/addons/crud/app/views/layouts/application/_nav_bar_top.html.slim
Show resolved
Hide resolved
0243e3d
to
179f653
Compare
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.
LGTM! 🚀
What happened 👀
import
lines in the root application.scss fileInsight 📝
Important
The purpose of this PR is to get a starting point, not to get the perfect layout 👍
The
look and feel
can be discussed in future issues, I also asked UX team as they have a Figma project for default Rails/Bootstrap apps, but they don't have recommendations fororganism components
(e.g. full menu, sidebar, etc...).=> I decided to use the Bootstrap examples from the official doc, so I don't need to think too much about it.
SCSS classes (instead of Bootstrap utility classes) will be implemented in this issue:
Proof Of Work 📹
The Top navbar is responsive, not the sidebar (yet?):