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

Admin cleanup #1339

Merged
merged 18 commits into from
Jan 14, 2018
Merged

Admin cleanup #1339

merged 18 commits into from
Jan 14, 2018

Conversation

tvdeyen
Copy link
Member

@tvdeyen tvdeyen commented Jan 5, 2018

A set of commits that stacked up while working on an Admin UI refresh

  • Unify flash and message styles
  • Remove %rounded-border sass extend
  • Remove old sassy buttons gradient image
  • Set width of unstyled medium selectbox
  • Remove old and unused CSS rules
  • Remove Bourbon

and more

Best reviewed commit by commit

@tvdeyen tvdeyen self-assigned this Jan 6, 2018
@tvdeyen tvdeyen changed the title Admin cleanup WIP Admin cleanup Jan 6, 2018
@tvdeyen tvdeyen changed the title WIP Admin cleanup Admin cleanup Jan 8, 2018
@tvdeyen tvdeyen removed their assignment Jan 8, 2018
@tvdeyen tvdeyen force-pushed the admin-cleanup branch 2 times, most recently from 735e7ef to 99b0032 Compare January 9, 2018 11:08
Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

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

One question, otherwise this looks like superb cleanup work.

Does this PR get rid of the Bourbon deprecations when precompiling assets?

@@ -29,21 +29,6 @@ table {
}
}

.floatThead-container {
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we use this class anywhere in our HTML?

Copy link
Member Author

Choose a reason for hiding this comment

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

No. The plugin that gets removed with this commit adds this DOM node.

@tvdeyen
Copy link
Member Author

tvdeyen commented Jan 9, 2018

@mamhoff

Does this PR get rid of the Bourbon deprecations when precompiling assets?

Good question. Didn’t checked that. Will do. Bourbon is actually not necessary anymore.

@tvdeyen tvdeyen force-pushed the admin-cleanup branch 2 times, most recently from 5f14f57 to 900520f Compare January 12, 2018 22:03
Before select2 replaces the selectbox the width is set to 100%, which leads to jumpy UI.
This is a left over from the Sassy buttons removal in a737e5c.
Use plain old border-radius instead.
Flash messages and info/warning messages used to have different colors. They now have a unified look.
These css rules are not used anymore.
These rules are very old not used in a long time
@tvdeyen
Copy link
Member Author

tvdeyen commented Jan 13, 2018

@mamhoff removed Bourbon as well. This should be ready to go. Then I can continue working with my other PR

This is a terrible hack for sticky table headers. The benefit is debatable and the problems is causes not worth it.
Use `render_message` helper instead of a plain `<p>` tag.
This is not necessary and causes overflowing
We never used much of Bourbon besides the border-radius and clearfix shorthands.
@tvdeyen
Copy link
Member Author

tvdeyen commented Jan 13, 2018

Added changelog entries for the removals

@tvdeyen tvdeyen mentioned this pull request Jan 13, 2018
Copy link
Contributor

@mamhoff mamhoff left a comment

Choose a reason for hiding this comment

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

Thank you!

@tvdeyen tvdeyen merged commit c595e29 into AlchemyCMS:master Jan 14, 2018
@tvdeyen tvdeyen mentioned this pull request Jan 15, 2018
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

Successfully merging this pull request may close these issues.

2 participants