Refactor, multiple crates and warnings fixes #113

Merged
sb10q merged 1 commits from esavkin/web2019:96-refactor into master 2024-08-17 17:37:18 +08:00
Member

Closes #96
Closes #94
Closes #108
Closes #76

Additional enhancements:

  • eliminated prop-drilling anti-pattern
  • optimized to be less rerenders (was: every card highlight/options/movement triggered full shop rerenders)
  • eliminated long hot-messy functions in random places - all logic is in shop_store.js (except options)
  • less memory consumption (up to 20%) for the same set of 9 cards
  • added legend to help users discover cards options
  • added red notification badge on unviewed options, pushing users to open and check the options
  • added tooltip that shows on card addition for 5 secs, and dismissable on next click/touch
  • added conditional compilation for debug purposes
  • Warnings and resources became generic, allowing to add new types later
  • added spare cards constant crate at the end
  • added "copy to clipboard button"
Closes #96 Closes #94 Closes #108 Closes #76 Additional enhancements: * eliminated prop-drilling anti-pattern * optimized to be less rerenders (was: every card highlight/options/movement triggered full shop rerenders) * eliminated long hot-messy functions in random places - all logic is in shop_store.js (except options) * less memory consumption (up to 20%) for the same set of 9 cards * ~~added legend to help users discover cards options~~ * ~~added red notification badge on unviewed options, pushing users to open and check the options~~ * added tooltip that shows on card addition for 5 secs, and dismissable on next click/touch * added conditional compilation for debug purposes * Warnings and resources became generic, allowing to add new types later * added spare cards constant crate at the end * added "copy to clipboard button"
esavkin force-pushed 96-refactor from b707b037b1 to 3e74727282 2024-01-05 17:34:01 +08:00 Compare
esavkin changed title from WIP: Refactor, multiple crates and warnings fixes to Refactor, multiple crates and warnings fixes 2024-01-08 17:20:12 +08:00
esavkin changed title from Refactor, multiple crates and warnings fixes to WIP: Refactor, multiple crates and warnings fixes 2024-01-08 17:25:36 +08:00
esavkin changed title from WIP: Refactor, multiple crates and warnings fixes to Refactor, multiple crates and warnings fixes 2024-01-08 17:42:25 +08:00
Owner

Is this tested? I just get a blank order page when running Zola.image

Is this tested? I just get a blank order page when running Zola.![image](/attachments/8ef02b34-906a-4cce-8a56-9dc18308282e)
Author
Member

Is this tested? I just get a blank order page when running Zola.

Yes and I just checked on the VM with Edge and it is working fine. What's the log and which revision did you pull?

> Is this tested? I just get a blank order page when running Zola. Yes and I just checked on the VM with Edge and it is working fine. What's the log and which revision did you pull?
esavkin force-pushed 96-refactor from 26601b7a9d to fcf0f49816 2024-01-09 10:15:48 +08:00 Compare
Owner

Git 2d6cb872f1

curl https://git.m-labs.hk/M-Labs/web2019/pulls/113.diff | patch -p1

Still results in empty page.

Git 2d6cb872f11417a5fdb7be7654e9124d5bc9af48 ``` curl https://git.m-labs.hk/M-Labs/web2019/pulls/113.diff | patch -p1 ``` Still results in empty page.
Owner

Problem not present when using your branch for some reason. Gitea bug?

Problem not present when using your branch for some reason. Gitea bug?
Owner

It's not a crate
image

It's not a crate ![image](/attachments/2f3f1bac-0065-489b-ab3c-e521d30822d5)
7.9 KiB
Owner

And clicking that "delete crate" button makes it impossible to add spare cards afterwards.

And clicking that "delete crate" button makes it impossible to add spare cards afterwards.
Owner

Functionality of "Remove all cards" looks dubious when there are multiple crates.
image

Functionality of "Remove all cards" looks dubious when there are multiple crates. ![image](/attachments/d88ada70-bb17-4929-b187-a50e4b3d118f)
Owner

Crate numbers should be also in the bottom list. Otherwise it's hard to tell which is which.
image

Crate numbers should be also in the bottom list. Otherwise it's hard to tell which is which. ![image](/attachments/4fd2f7dc-cda2-45df-b3ae-e20bc93e70e1)
Owner

Remove this red dot entirely. I don't see how to make it accurate other than have the user check a box "I configured this card", which sounds too cumbersome.
image

Remove this red dot entirely. I don't see how to make it accurate other than have the user check a box "I configured this card", which sounds too cumbersome. ![image](/attachments/bbc29ea6-f09f-4daf-ae33-a14cc9ab1c94)
Owner

Text at the top should now be expanded horizontally.
image

Text at the top should now be expanded horizontally. ![image](/attachments/680ace95-8ab6-4e7b-b9a9-27d92c7157e1)
Author
Member

Remove this red dot entirely. I don't see how to make it accurate other than have the user check a box "I configured this card", which sounds too cumbersome.

Between webiste being slightly annoying (actually not really, most shops will not proceed unless you choose configuration) in forcing the configuration, and making the sales manager do the same, I would prefer the first without doubts.

Another suggestion is to show a tooltip at options when you add the card. The tooltip can be dismissed on next click/touch inside and outside that tooltip.
image

> Remove this red dot entirely. I don't see how to make it accurate other than have the user check a box "I configured this card", which sounds too cumbersome. Between webiste being slightly annoying (actually not really, most shops will not proceed unless you choose configuration) in forcing the configuration, and making the sales manager do the same, I would prefer the first without doubts. Another suggestion is to show a tooltip at options when you add the card. The tooltip can be dismissed on next click/touch inside and outside that tooltip. ![image](/attachments/37ea46cc-d4f0-4ff1-ab4c-4e414b26700e)
Owner

making the sales manager do the same

We just introduced this config button recently. Are people not using it?

> making the sales manager do the same We just introduced this config button recently. Are people not using it?
Author
Member

Are people not using it?

We don't have analytics, so the data cannot be accurate, but from orders I've seen feels like it is being mostly undiscovered unless manager replies to them to check the configurations.

> Are people not using it? We don't have analytics, so the data cannot be accurate, but from orders I've seen feels like it is being mostly undiscovered unless manager replies to them to check the configurations.
Owner

Then try adding a tooltip (which disappears with a timer or click) that draws attention to the configuration button.

Then try adding a tooltip (which disappears with a timer or click) that draws attention to the configuration button.
esavkin added 3 commits 2024-01-15 15:51:10 +08:00
Author
Member

Quote generator was also updated, so it is ready for the upcoming changes

Quote generator was also updated, so it is ready for the upcoming changes
Owner

The "USD 0.00" next to "Spare cards" does not add anything and is slightly misleading.
image

The "USD 0.00" next to "Spare cards" does not add anything and is slightly misleading. ![image](/attachments/211a6316-007e-40ca-8eb0-e94e39f911f2)
Owner

The "i" icon doesn't do anything. Remove.
image

The "i" icon doesn't do anything. Remove. ![image](/attachments/a309b0d2-2bfc-4cee-b4ce-ce66b8780434)
Owner

"Setup card's configuration by tapping at the top of the card" needs to be reworded and grammar fixed

"Setup card's configuration by tapping at the top of the card" needs to be reworded and grammar fixed
Owner

Warnings symbols get misaligned because of the config buttons. The warning symbols should probably be removed from there anyway, just like the "i" icons.
image

Warnings symbols get misaligned because of the config buttons. The warning symbols should probably be removed from there anyway, just like the "i" icons. ![image](/attachments/2ff4b2cd-6644-42f8-a33c-5841a3aca6e6)
Owner

"Reset the order" should add power moduel and Kasli, not just an empty crate. And that crate should get opened in the view.

"Reset the order" should add power moduel and Kasli, not just an empty crate. And that crate should get opened in the view.
Owner

Or better: just remove "Reset the order" entirely. Not sure what the use case is. And people can just refresh the page instead anyway.

Or better: just remove "Reset the order" entirely. Not sure what the use case is. And people can just refresh the page instead anyway.
esavkin added 2 commits 2024-01-23 11:44:16 +08:00
9d5ed66985 Do not show reminders in summary
Signed-off-by: Egor Savkin <es@m-labs.hk>
Author
Member

Warnings symbols get misaligned because of the config buttons. The warning symbols should probably be removed from there anyway, just like the "i" icons.

Since there are multiple crates, and only one can be open at the moment, the user may accidentally skip the card with issues.
For now it shows only warnings, and the alignment has been fixed.

> Warnings symbols get misaligned because of the config buttons. The warning symbols should probably be removed from there anyway, just like the "i" icons. Since there are multiple crates, and only one can be open at the moment, the user may accidentally skip the card with issues. For now it shows only warnings, and the alignment has been fixed.
Owner

Not sure how you managed that result, but the warning on kasli is now broken and replaced with an "i" icon.
image

Not sure how you managed that result, but the warning on kasli is now broken and replaced with an "i" icon. ![image](/attachments/85179f08-54ea-4ed6-ac91-0ee5191c3a4a)
Owner

Same problem on Kasli-SoC.

Same problem on Kasli-SoC.
esavkin added 1 commit 2024-01-23 16:30:31 +08:00
3366f80ed7 Fix reminder background color
Signed-off-by: Egor Savkin <es@m-labs.hk>
Author
Member

Not sure how you managed that result, but the warning on kasli is now broken and replaced with an "i" icon.

It is not warning, but a reminder, in the same way as it was originally, but moved from crate-level to card-level. Background color for overlay fixed.

> Not sure how you managed that result, but the warning on kasli is now broken and replaced with an "i" icon. It is not warning, but a reminder, in the same way as it was originally, but moved from crate-level to card-level. Background color for overlay fixed.
sb10q merged commit 3366f80ed7 into master 2024-01-23 16:48:54 +08:00
Sign in to join this conversation.
No reviewers
No Label
No Milestone
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: M-Labs/web2019#113
No description provided.