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

Changes for supermarket banner for announcements #2306

Merged
merged 4 commits into from
Oct 25, 2021

Conversation

msys-sgarg
Copy link
Contributor

@msys-sgarg msys-sgarg commented Oct 19, 2021

Signed-off-by: smriti sgarg@msystechnologies.com

Description

We have added a new partial to render banner for supermarket announcements.
Also introduced once env variable which when defined will be rendered on supermarket main page. This will also help to update the banner text without requiring a code change in future.

Also introduced specs for application.html.erb

Issues Resolved

#2292

Check List

@msys-sgarg msys-sgarg requested review from a team as code owners October 19, 2021 10:53
@msys-sgarg msys-sgarg force-pushed the smriti/2292_banner_about_maintenance_window branch from df6ea2f to e6b75f6 Compare October 19, 2021 10:58
@github-actions
Copy link

Simplecov Report

Covered Threshold
98.29% 90%

@msys-sgarg
Copy link
Contributor Author

Signed-off-by: smriti <sgarg@msystechnologies.com>

div id documented for better understanding

Signed-off-by: smriti <sgarg@msystechnologies.com>
@msys-sgarg msys-sgarg force-pushed the smriti/2292_banner_about_maintenance_window branch from 048995d to 5f2a10a Compare October 19, 2021 11:54
…g partial

Signed-off-by: smriti <sgarg@msystechnologies.com>
Signed-off-by: smriti <sgarg@msystechnologies.com>
src/supermarket/app/assets/stylesheets/alerts.scss Outdated Show resolved Hide resolved
@@ -20,7 +20,7 @@
<% end %>
<%= csrf_meta_tags %>
<% unless air_gapped? %>
<%= render 'analytics' %>
<%= render partial: 'application/analytics' %>
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 really need partial, i think it will work without partial
<%= render 'application/analytics' %>

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It can.. but adding partial makes it more readable in my opinion

Signed-off-by: smriti <sgarg@msystechnologies.com>
@saghoshprogress saghoshprogress merged commit 4c92c35 into main Oct 25, 2021
@saghoshprogress saghoshprogress deleted the smriti/2292_banner_about_maintenance_window branch October 25, 2021 12:36
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.

None yet

2 participants