[IMP] website_sale: add location_selector raw design #3087

Brieuc-brd · 2024-04-12T13:15:27Z

task-3795332

robodoo · 2024-04-12T13:16:15Z

This PR targets the un-managed branch odoo-dev/odoo:master-delivery_location_selector-vcr, it needs to be retargeted before it can be merged.

stefanorigano · 2024-04-13T06:19:24Z

..._sale/static/src/js/location_selector/location_selector_dialog/location_selector_dialog.scss

+    border-bottom: $border-width solid $primary;
+}
+
+.modal-footer,


@Brieuc-brd , these lines will affect the entire frontend ^^ ... Let's scope the entire file, thanks 👍

@stefanorigano Indeed 👍
Since there's no easy way to add a custom class to the <Dialog/> (I wanted to avoid using xpath), I wrapped the component into a div.o_locationSelector.
What do you think?

Ok to me, it suits the current needs.
The goal is indeed to style this backbone layout, the entire feature is not ready for production just yet, we'll see how to improve this specific part during the "real implementation".

What do you mean there is no easy way to add a custom class to the <Dialog/>? What about contentClass= attribute on the XML element?

Good catch @chevalierv ! I don't know why, I thought contentClass only targeted the body of the modal 🤔 .
It's much better using it, thanks! 👍

chevalierv · 2024-04-17T11:33:16Z

...e_sale/static/src/js/location_selector/location_selector_dialog/location_selector_dialog.xml

+                                    <div class="collapse show" t-attf-id="collapseLocationInfosMobile_{{this.state.selectedLocationId.toString()}}">
+                                        <button class="collapsed d-flex gap-2 gap-lg-3 w-100 border-0 p-3 bg-transparent text-start" type="button" t-attf-data-bs-target="#map_collapseHours_{{props.id}}" aria-expanded="false" t-attf-aria-controls="map_collapseHours_{{props.id}}" data-bs-toggle="collapse">
+                                            <strong class="o_locationSelector_number fw-bold">6</strong>
+                                            <span class="d-flex flex-column gap-2 w-100">
+                                                <span class="d-flex flex-column">
+                                                    <strong class="fw-bold">Librairie l’as-tu lu Jauche</strong>
+                                                    <small class="text-muted">Av. Albert Drossart 323, 1350 Orp-Jauche</small>
+                                                </span>
+                                                <span class="d-flex align-items-center gap-1 small fw-bold">
+                                                    <i class="fa fa-clock-o" role="img"/>
+                                                    Opening hours
+                                                    <i class="o_locationSelector_hours_caret fa fa-caret-up ms-auto transition-base"/>
+                                                </span>
+                                                <span class="collapse" t-attf-id="map_collapseHours_{{props.id}}">
+                                                    <span class="o_locationSelector_schedule d-grid gap-1 border rounded p-2 bg-white">
+                                                        <span class="d-contents">
+                                                            <small class="me-4 text-muted">Monday</small>
+                                                            <span class="o_locationSelector_schedule_hours d-flex flex-wrap">
+                                                                <small>06:30 - 12:00</small>
+                                                            </span>
+                                                        </span>
+                                                        <span class="d-contents">
+                                                            <small class="me-4 text-muted">Tuesday</small>
+                                                            <span class="o_locationSelector_schedule_hours d-flex flex-wrap">
+                                                                <small>06:30 - 12:00</small>
+                                                                <small>13:00 - 19:30</small>
+                                                            </span>
+                                                        </span>
+                                                        <span class="d-contents">
+                                                            <small class="me-4 text-muted">Wednesday</small>
+                                                            <span class="o_locationSelector_schedule_hours d-flex flex-wrap">
+                                                                <small>06:30 - 12:00</small>
+                                                                <small>13:00 - 19:30</small>
+                                                            </span>
+                                                        </span>
+                                                        <span class="d-contents">
+                                                            <small class="me-4 text-muted">Thursday</small>
+                                                            <span class="o_locationSelector_schedule_hours d-flex flex-wrap">
+                                                                <small>06:30 - 12:00</small>
+                                                                <small>13:00 - 19:30</small>
+                                                            </span>
+                                                        </span>
+                                                        <span class="d-contents">
+                                                            <small class="me-4 text-muted">Friday</small>
+                                                            <span class="o_locationSelector_schedule_hours d-flex flex-wrap">
+                                                                <small>13:00 - 19:30</small>
+                                                            </span>
+                                                        </span>
+                                                        <span class="d-contents">
+                                                            <small class="me-4 text-muted">Saturday</small>
+                                                            <span class="o_locationSelector_schedule_hours d-flex flex-wrap">
+                                                                <small class="text-danger">Closed</small>
+                                                            </span>
+                                                        </span>
+                                                        <span class="d-contents">
+                                                            <small class="me-4 text-muted">Sunday</small>
+                                                            <span class="o_locationSelector_schedule_hours d-flex flex-wrap">
+                                                                <small class="text-danger">Closed</small>
+                                                            </span>
+                                                        </span>
+                                                    </span>
+                                                </span>
+                                                <!-- TODO: Toggle "Open now/Close now" elements depending on the time -->
+                                                <small class="d-md-none text-success fw-bold">Open now</small>
+                                                <!-- <small class="d-md-none text-danger fw-bold">Close now</small> -->
+                                            </span>
+                                        </button>
+                                    </div>


IMO this shouldn't be a collapse as the first location will always be selected anyway (it should already be the case but maybe there is a bug on my part). Also, to reduce the code, could you move that part to the Map component?

Ok I've removed the collapse and moved the code into the map view 👍 .
I also took the opportunity to clean up and further simplify the overall xml structure.

My only concern is that this button doesn't work anymore since I moved it into the map view.
Could you take a look at it? I've left you a few comments in the code 👇
https://github.com/odoo-dev/odoo/pull/3087/files#diff-137b82b9720f2631064e69e4e15ed7e89bb15ebe7b528af60d9809ff0d6265d3R16-R19

addons/website_sale/static/src/js/location_selector/map/map.js

addons/website_sale/static/src/js/location_selector/map/map.scss

chevalierv · 2024-04-17T11:39:43Z

addons/website_sale/static/src/js/location_selector/map/map.xml

@@ -1,12 +1,12 @@
 <?xml version="1.0" encoding="UTF-8" ?>
 <templates xml:space="preserve">
    <t t-name="website_sale.locationSelector.map">
-        <div class="w-100 h-100" style="width: 30rem! important; height: 30rem !important;" t-ref="mapContainer"></div>
+        <div class="w-100 flex-grow-1" t-ref="mapContainer"/>


I don't think this would work, it's maybe why it fails in mobile...

According to leaflet quickstart guide, the map container needs to have a defined height.

I've done a few tests and even with a defined height, the card still doesn't work on mobile.
Something seems to break the map when the "md and below" breakpoint is triggered 🤔 .
Could you have a look at it ?

Selectionne.avril.17.2024.15.46.11.mp4

I think the problem is that the map is rendered when the div is not shown, and then since we use bootstrap, I can't catch easily any event in javascript to inform the map that its size has been updated.

I'll try to drop the use of tab-pane and do the switch in owl instead. If that doesn't work, I'll go back to good old js to catch the event

chevalierv · 2024-04-17T11:40:14Z

addons/website_sale/static/src/js/location_selector/map/map.xml

    </t>

    <t t-name="website_sale.locationSelector.map.marker">
-        <div style="color: var(--primary);">
-            <svg version="1.1" id="Layer_1" xmlns="http://www.w3.org/2000/svg" x="0px" y="0px" viewBox="0 0 61 78.9" style="enable-background:new 0 0 61 78.9;" xml:space="preserve">
+        <div style="color: var(--light);"> <!-- todo vcr change var to primary on click -->


on click? do you mean when the location is selected?

Yes, indeed! 😅
I've cleaned up the map view. All the style is already defined for the selected marker. We just need to add the o_location_selector_marker_icon_selected class to the selected marker. Could you have a look at it ?
https://github.com/odoo-dev/odoo/pull/3087/files#diff-654f2c8dd7477a3a02004b01f1a588a4a982ea28f8ff607674e6130d4867fa4e

Not possible IMO, the marker shall all have the same design.

So it's not possible to change the color of the selected marker?

It won't work the way it's technically done right now but I'll try to adapt the code to make it works

Brieuc-brd · 2024-04-18T12:27:21Z

Hey @chevalierv 👋
Thanks for your review 🙂
I've updated the template and left you a few comments, could you have a look at it ?

Brieuc-brd · 2024-04-25T06:31:44Z

@chevalierv 👋
Little reminder :)

chevalierv · 2024-04-29T07:52:38Z

addons/website_sale/static/src/js/location_selector/location/location.xml

+        <button
+            class="collapsed list-group-item list-group-item-action d-flex gap-2 gap-lg-3"


Are those changes needed? What is the use of collapsed and button ?

Suggested change

<button

class="collapsed list-group-item list-group-item-action d-flex gap-2 gap-lg-3"

<div

class="list-group-item list-group-item-action d-flex gap-2 gap-lg-3"

Indeed, collapsed wasn't necessary, I removed it 👍
But button follows the semantics provided by the Bootstrap component.

addons/website_sale/static/src/js/location_selector/location/location.xml

chevalierv · 2024-04-29T07:54:40Z

addons/website_sale/static/src/js/location_selector/location/location.xml

+
+                <!-- Schedule -->
+                <span class="collapse d-md-none" t-attf-id="collapseHours_{{props.id}}" data-bs-parent="#o_location_selector_list_view">
+                    <t t-call="website_sale.locationSelector.schedule"/>


You can't call a template of another component. I'll create a new owl component if you need to use the schedule at multiple places.

It would be nice, thanks 👍

...e_sale/static/src/js/location_selector/location_selector_dialog/location_selector_dialog.xml

chevalierv · 2024-04-29T11:47:19Z

Hello @Brieuc-brd,

I'm starting to update my branches to enable the features you need in your branch (like the opening hours). I've seen that you've created a branch in an enterprise. As no changes were made, I've deleted it as it causes runbot to be red. Please use my branch directly if you don't need to change anything.

The hours are dynamic but there is a problem when the pickup point has only one time frame: the opening hours are too much on the right. Can you take a quick look at that?

At the same time, I'm fixing the others point like the map and the choose location button.

task-3795332

This commit will be divided into the base branch to ease the review of the design team.

stefanorigano reviewed Apr 13, 2024

View reviewed changes

Brieuc-brd force-pushed the master-delivery_location_selector-vcr-morm-brd branch 2 times, most recently from 01d3b07 to b8b12f3 Compare April 15, 2024 14:02

chevalierv reviewed Apr 17, 2024

View reviewed changes

addons/website_sale/static/src/js/location_selector/map/map.js Outdated Show resolved Hide resolved

chevalierv reviewed Apr 17, 2024

View reviewed changes

addons/website_sale/static/src/js/location_selector/map/map.scss Outdated Show resolved Hide resolved

chevalierv reviewed Apr 17, 2024

View reviewed changes

Brieuc-brd force-pushed the master-delivery_location_selector-vcr-morm-brd branch 3 times, most recently from 2f228b5 to 09490bb Compare April 18, 2024 10:49

Brieuc-brd requested a review from chevalierv April 25, 2024 06:31

chevalierv force-pushed the master-delivery_location_selector-vcr-morm-brd branch from 09490bb to 8559aee Compare April 29, 2024 08:09

chevalierv requested changes Apr 29, 2024

View reviewed changes

Brieuc-brd force-pushed the master-delivery_location_selector-vcr-morm-brd branch from 8559aee to 08ed69f Compare April 29, 2024 10:21

Brieuc-brd requested a review from chevalierv April 29, 2024 10:30

chevalierv force-pushed the master-delivery_location_selector-vcr branch 3 times, most recently from cacd594 to 0e14844 Compare April 29, 2024 11:35

chevalierv force-pushed the master-delivery_location_selector-vcr-morm-brd branch from 08ed69f to 141cb6f Compare April 29, 2024 11:37

chevalierv force-pushed the master-delivery_location_selector-vcr-morm-brd branch 2 times, most recently from 9ed217f to 53d73de Compare April 29, 2024 11:49

chevalierv force-pushed the master-delivery_location_selector-vcr branch from 0e14844 to 10ce3b7 Compare April 29, 2024 15:48

chevalierv force-pushed the master-delivery_location_selector-vcr-morm-brd branch from 53d73de to 7c2304d Compare April 29, 2024 15:49

chevalierv force-pushed the master-delivery_location_selector-vcr branch 2 times, most recently from 8e13a69 to 5ced43d Compare April 30, 2024 15:33

chevalierv force-pushed the master-delivery_location_selector-vcr branch 2 times, most recently from f5057d3 to 51361c4 Compare April 30, 2024 15:50

chevalierv force-pushed the master-delivery_location_selector-vcr-morm-brd branch from 7c2304d to 86107a3 Compare April 30, 2024 15:55

[IMP] website_sale: add location_selector raw design

807d54d

task-3795332

chevalierv force-pushed the master-delivery_location_selector-vcr-morm-brd branch from 86107a3 to 807d54d Compare April 30, 2024 16:30

Various improvments about design and functionnality

2401dce

This commit will be divided into the base branch to ease the review of the design team.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[IMP] website_sale: add location_selector raw design #3087

[IMP] website_sale: add location_selector raw design #3087

Brieuc-brd commented Apr 12, 2024

robodoo commented Apr 12, 2024

stefanorigano Apr 13, 2024

Brieuc-brd Apr 15, 2024

stefanorigano Apr 15, 2024

chevalierv Apr 17, 2024

Brieuc-brd Apr 18, 2024

chevalierv Apr 17, 2024 •

edited

Brieuc-brd Apr 18, 2024

chevalierv Apr 17, 2024

Brieuc-brd Apr 18, 2024

chevalierv Apr 29, 2024

chevalierv Apr 17, 2024

Brieuc-brd Apr 18, 2024

chevalierv Apr 26, 2024

Brieuc-brd Apr 26, 2024

chevalierv Apr 29, 2024

Brieuc-brd commented Apr 18, 2024

Brieuc-brd commented Apr 25, 2024

chevalierv Apr 29, 2024

Brieuc-brd Apr 29, 2024

chevalierv Apr 29, 2024

Brieuc-brd Apr 29, 2024

chevalierv commented Apr 29, 2024

		<button
		class="collapsed list-group-item list-group-item-action d-flex gap-2 gap-lg-3"

[IMP] website_sale: add location_selector raw design #3087

Are you sure you want to change the base?

[IMP] website_sale: add location_selector raw design #3087

Conversation

Brieuc-brd commented Apr 12, 2024

robodoo commented Apr 12, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chevalierv Apr 17, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Brieuc-brd commented Apr 18, 2024

Brieuc-brd commented Apr 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

chevalierv commented Apr 29, 2024

chevalierv Apr 17, 2024 •

edited