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.

Sign up for GitHub

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

Jump to bottom

Minimal bento elements for Staging #8269

Merged
merged 8 commits into from
Sep 8, 2019
Merged

Minimal bento elements for Staging #8269

merged 8 commits into from
Sep 8, 2019

Conversation

saraycp
Copy link
Contributor

@saraycp saraycp commented Sep 4, 2019 •
edited
Loading

Since we have migrated almost all our views to Bootstrap and they are well tested, we started to remove most of the bento related elements.

However, the Staging dashboard is not migrated to Bootstrap and we decided to preserve a minimal amount of bento elements to keep the dashboard working. They are moved to obs_factory namespace in views (including the layout), assets, controllers, etc.

Most of the old CSS, Javascript and images/icon files has been removed, for that reason, icons._sprite.png was updated and now it only contains the remaining icons and the sponsor ones. To do so, I had to modify the task (sprite.rake) used to generate this image based on the icon files. It looked like this before:

before

and now it looks like:

icons_sprite

To test this PR, please, follow this instructions to create content locally or try out on the review-app. The views to test are:

@saraycp saraycp added Frontend Things related to the OBS RoR app review-app Apply this label if you want a review app started labels Sep 4, 2019
@obs-bot
Copy link
Collaborator

obs-bot commented Sep 4, 2019

Review app will appear here: http://obs-reviewlab.opensuse.org/saraycp-minimal_bento_for_staging

hennevogel
hennevogel reviewed Sep 4, 2019
View reviewed changes
Copy link
Member

@hennevogel hennevogel left a comment

Choose a reason for hiding this comment

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

One general question: Why do you move application.css.erb to application.css.erb if there is no erb in it?

@hennevogel
Copy link
Member

In the last commit you also remove the sponsor images again.

@saraycp
Copy link
Contributor Author

saraycp commented Sep 4, 2019

@hennevogel

One general question: Why do you move application.css.erb to application.css.erb if there is no erb in it?

Good catch, updated!

In the last commit you also remove the sponsor images again.

We had sponsor images in two different places: inside assets/icons/directory and inside assets/images/sponsor/ directory. AFAIK, we are only using those in assets/icons/(now moved to assets/images/icons) that are used to generate icons_sprite.png. So I removed all the logos inside assets/images/sponsor/despite it contained some logos that weren't in assets/icons/, but I checked they weren't used in production either.

DavidKang
DavidKang suggested changes Sep 5, 2019
View reviewed changes
Copy link
Contributor

@DavidKang DavidKang left a comment •
edited
Loading

Choose a reason for hiding this comment

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

You miss a change in config/initializer/assets.rb. You changed the extension of the application.css.erb to application.scss, so the assets will not be precompiled. If I'm not wrong the correct extension should be .css.scss

@saraycp
Copy link
Contributor Author

saraycp commented Sep 5, 2019

You miss a change in config/initializer/assets.rb. You changed the extension of the application.css.erb to application.scss, so the assets will not be precompiled. If I'm not wrong the correct extension should be .css.scss

You are right, thanks. Updated!

hennevogel
hennevogel requested changes Sep 5, 2019
View reviewed changes
Copy link
Member

@hennevogel hennevogel left a comment

Choose a reason for hiding this comment

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

If I'm not wrong the correct extension should be .css.scss

You are wrong, manifest files don't have other preprocessor extensions.

@DavidKang
Copy link
Contributor

You are wrong, manifest files don't have other preprocessor extensions.

ok, sorry @saraycp

@saraycp
Copy link
Contributor Author

saraycp commented Sep 5, 2019 •
edited
Loading

Ok, @hennevogel, moved application.css.scss back to application.scss and also rebased.
Please 👀

@saraycp saraycp force-pushed the minimal_bento_for_staging branch 2 times, most recently from 3587725 to 2daeb9d Compare September 5, 2019 19:38
@hennevogel
Copy link
Member

Sorry now you removed the wrong extension. This should be application.css

saraycp and others added 8 commits September 6, 2019 15:33
@saraycp
Copy layout and CSS files into obs_factory namespace
97ee9c3 
Layout view, partials and CSS are now inside webui/obs_factory.
The layout is now called webui/obs_factory/application.html.erb.

The original files will be removed in following commits.

Co-authored-by: David Kang <dkang@suse.com>
@saraycp
Copy JavaScript files into obs_factory namespace
fd14739 
In the copy we remove code that for sure is not going to be used in obs_factory.

The original files are not removed yet.
@saraycp
Remove all unused bento CSS
515a52e
@saraycp
Remove all unused bento JavaScript
298752c
@saraycp
Modify the task sprites.rake
b65bfed 
It generates the sprite image based on the icons.
@saraycp
Remove unsused icons and re-generate icons_sprite image
d7d59b3 
Move assets/icons/ to /assets/images/icons.
Remove most of the icons except the sponsors ones and those used in
obs_factory.

Use the task lib/tasks/sprites.rake to re-generate icons_sprite.png and
its corresponding CSS file (icons.scss).
@saraycp
Remove unused images keeping obs_factory ones
43038da
@saraycp
Rename paths to session actions in obs_factory
c8ce5eb
@saraycp
Copy link
Contributor Author

saraycp commented Sep 6, 2019

oh sorry @hennevogel 😅 Updated!

@hennevogel hennevogel merged commit 32a1567 into openSUSE:master Sep 8, 2019
@saraycp saraycp deleted the minimal_bento_for_staging branch September 10, 2019 13:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hennevogel hennevogel hennevogel approved these changes

@DavidKang DavidKang DavidKang approved these changes

Assignees
No one assigned
Labels
Frontend Things related to the OBS RoR app review-app Apply this label if you want a review app started
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@saraycp @obs-bot @hennevogel @DavidKang