-
Notifications
You must be signed in to change notification settings - Fork 163
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
Mwpw 157587 | [Geo-Routing][Page Performance] Removing the georouting modal background image #2891
Changes from all commits
05b5b8b
b4f1ba9
ee3f9e1
5874ef5
73e536a
8dd56c1
3e055a4
f730d82
65a40a2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -21,10 +21,6 @@ | |
z-index: -1; | ||
} | ||
|
||
.dialog-modal.locale-modal-v2 .georouting-bg svg { | ||
width: 100%; | ||
} | ||
|
||
.dialog-modal.locale-modal-v2 .picker { | ||
position: fixed; | ||
margin-top: 2px; | ||
|
@@ -140,19 +136,6 @@ | |
padding: initial; | ||
} | ||
|
||
@media (min-width: 480px) { | ||
.dialog-modal.locale-modal-v2 { | ||
background-image: url('/libs/features/georoutingv2/img/GeoModal_BG_Map_Tablet.png'); | ||
background-position: center; | ||
background-repeat: no-repeat; | ||
background-size: cover; | ||
} | ||
|
||
.dialog-modal.locale-modal-v2 .georouting-bg { | ||
display: none; | ||
} | ||
} | ||
|
||
@media (min-width: 600px) { | ||
.dialog-modal.locale-modal-v2 .georouting-wrapper { | ||
padding: 56px 56px 40px; | ||
|
@@ -168,9 +151,3 @@ | |
max-width: 720px; | ||
} | ||
} | ||
|
||
@media (min-width: 1200px) { | ||
.dialog-modal.locale-modal-v2 { | ||
background-image: url('/libs/features/georoutingv2/img/GeoModal_BG_Map_Desktop.png'); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. we need to remove the image from assets There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @narcis-radu I have removed this image from repo |
||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -228,13 +228,7 @@ function buildContent(currentPage, locale, geoData, locales) { | |
async function getDetails(currentPage, localeMatches, geoData) { | ||
const availableLocales = await getAvailableLocales(localeMatches); | ||
if (!availableLocales.length) return null; | ||
const { innerWidth } = window; | ||
let svgDiv = null; | ||
if (innerWidth < 480) { | ||
const { default: getMobileBg } = await import('./getMobileBg.js'); | ||
svgDiv = createTag('div', { class: 'georouting-bg' }, getMobileBg()); | ||
} | ||
const georoutingWrapper = createTag('div', { class: 'georouting-wrapper fragment', style: 'display:none;' }, svgDiv); | ||
const georoutingWrapper = createTag('div', { class: 'georouting-wrapper fragment', style: 'display:none;' }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @overmyheadandbody This would currently be required as the getDetails function that fetches and adds the modal to the DOM is called before showModal that fetches the georoutingv2.css and till that time the modal needs to be hidden. We would either need to add this as class to a different css file that gets loaded before and then apply this class here. @narcis-radu @mokimo @robert-bogos Could you share your inputs on the same? |
||
currentPage.url = window.location.hash ? document.location.href : '#'; | ||
if (availableLocales.length === 1) { | ||
const content = buildContent(currentPage, availableLocales[0], geoData); | ||
|
This file was deleted.
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.
we need to remove the image from assets
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.
@narcis-radu I have removed this image from repo