Make the seams labels into clickable links

[michaelbromley]

[dario-piotrowicz]

Regarding the seams I think they don't work too well when the cart is open, if you have them on and open the cart this is what you get:

Note that the seams labels are present but not the seams themselves (they are but behind the backdrop), also the menu seam label hides the cart close button which is sort of inconvenient (especially since the cart doesn't close when you click outside the side panel)

[gioboa]

How we can redesign this component to be usable?

[dario-piotrowicz]

@gioboa regarding the cart my suggestion would be to simply make sure that the cart sidebar has a higher z-index in respect to the seams labels (or actually, I think it would be very nice to use isolation: isolate to get separate stacking contexts so that you avoid potential issues with the z-indexes)

---

By the way, not to have the labels be too distracting (and completely covering elements on the page) I think you could make them less prominent, for example you could have them have the same (text) color as the seams and a white slightly opaque background as we did here: https://cloud-gallery.web-experiments.workers.dev/

Another version of the seams labels which we've have used is this one: https://productivity-suite.web-experiments.workers.dev/

Anyways these are just some ideas/suggestions :slightly_smiling_face:

[gioboa]

@dario-piotrowicz I like your solution. would you like open a PR for it?

[dario-piotrowicz]

sure :slightly_smiling_face: , I'll look into it and open a PR in the next few days :slightly_smiling_face::+1:

[dario-piotrowicz]

Hi there, I've opened a PR for a possible improvement for the seams/labels please have a look and let me know what you think :slightly_smiling_face:

anyways as I mentioned in the PR, I believe there is a fundamental issue on how the cart fragment has been implemented, let me explain what I mean:

the cart fragment doesn't occupy a real section of the screen as all it's contents are externalized via absolute or fixed positioning

so, you can see that the fragment is basically seen an empty inline block:

also its label is still there and overlaps with the next fragment's label:

given this implementation I do not believe there is a clean and truthful way of adding a label and seams to the fragment since the fragment really doesn't have a proper size, but leaks its content on the outside

Moreover you cannot see the cart in isolation:

I think that this implementation is not the correct one as it shows a wrong use of fragments (I also guess this can not work too well with out of order streaming?)

Personally I think that the cart icon should be part of the host, as well as the right sidebar and that the cart fragment should be a normal fragment which resides inside the sidebar, this would solve all the issues above (allowing us to have clear seams/labels for the cart).

Alternatively (what I did in my PR as a hopefully temporary solution) the cart implementation can stay the same and its seams/label are just being hidden (but then I am not sure what is the point of having it as a standalone fragment).

Other solutions could be found, like moving the seams/labels inside the cart fragment or something... but I feel like those would just be hacks that simply hide the real issue.

Anyways my PR is not making the cart issue any worse (it is actually making it slightly better since I am not adding that extra label on the page) so I think that should not block the PR but potentially be a followup. (it would also require some rewriting of the fragment and the host)

[dario-piotrowicz]

(actually I think that the cart menu icon button should be part of the menu fragment instead of having it somewhere else and hacking its placement)

[gioboa]

Mmm maybe we can add an attribute to the cart application to pass the label value so we define the value from the host. Does that make sense?

[dario-piotrowicz]

yes we can have everything be moved inside the cart in the sidebar so that we put the label and seams there, but then what about the cart icon button? do we put seams with a label there as well, just seams? nothing?

In any case we'd be lying to the user telling them that something is a fragment whilst it isn't, it would also make the code more messy and difficult to understand from an external point of view.

In any case as I mentioned I am sure various workaround can be found but they aren't the correct solution in my opinion.

Also I don't think that positioning the button with absolute positioning can be a robust/responsive solution and it will always cause weird behavior with different screen sizes and/or browser settings, for example we can immediately see this in the current implementation on slightly smaller screens, the menu doesn't make room for the button so thing start overlapping:

Also with different font-sizes the position starts to break: (but I can see other parts of the app also breaking anyways) We can fix this last issue with font-sizes by absolutely positioning using rems... but anyways this relies on the menu styles, if you change the padding in the menu or anything you'll have to remember to update the absolute positioning as well....

Moreover if we wanted to have a mobile view in which the menu becomes vertical (I think that something like this is going to be needed if this app needs to look ok on mobile screens) then aligning the absolute positioning with the different media query styles is going to be quite a headache....

Sorry for rumbling, anyways my point is that personally I think that workarounds are possible but if you want a nice non-hacky robust solution the only option is to change the underling implementation of the cart fragment as anything else is going to have serious downsides. I don't even think that changing the cart implementation should be an extreme amount of work, I guess it's just moving the jsx around a bit (hopefully). (Also I am not sure how much work is left on the UI of the page, but depending on that I have a strong feeling that refactoring the cart in the way I mentioned can take much less time and pain than trying to make the current version work acceptably well).

Anyways if you believe that spending time on it is not worth it or disagree with my concerns that's totally fine, I'm just providing my opinion on the matter :slightly_smiling_face:

[gioboa]

I think you're right. We need to support mobile devices with a responsive layout. You can go for it

[dario-piotrowicz]

@gioboa ah ok cool, I can give it a shot then :slightly_smiling_face:

---

By the way, if you could give me some suggestions on how to develop the application that would be really great :pray: In order make the seams changes I span up all the apps using npm run preview and then when I made some changes I had to kill processes and re-start them again to see the results in the browser, it is rather painful, do you have a better way to develop the application in a way what compilation happens on all changes (potentially with also hot reloading?)?