CleverCloud / clever-components

Collection of Web Components by Clever Cloud
https://www.clever-cloud.com/doc/clever-components/
Apache License 2.0
215 stars 19 forks source link

refactor(cc-tile-requests)!: rework properties to avoid impossible states #1057

Closed HeleneAmouzou closed 3 weeks ago

HeleneAmouzou commented 1 month ago

What does this PR do?

How to review?

github-actions[bot] commented 1 month ago

🔎 A preview has been automatically published : https://clever-components-preview.cellar-c2.services.clever-cloud.com/cc-tile-requests/state-migration/index.html.

This preview will be deleted once this PR is closed.

Galimede commented 1 month ago

Here are some thoughts on the empty state tile-metrics vs tile-requests:

Conclusion/TL;DR: In my opinion, there's no correct answer. However, I do like the cc-tile-metrics way as it's the smart responsibility to handle and calculate its state, the component should only apply its state and not calculate it, it's simply not its job.

Maybe, if we had to make a compromise we could do it the other way around first and when we'll have a smart someday, go for this solution. :man_shrugging:

I'm sorry @florian-sanders-cc and @HeleneAmouzou that's not a clear yes or no. :sweat_smile:

florian-sanders-cc commented 1 month ago

Sync about empty states

Considering @Galimede's inputs, we have decided that it made more sense to leave the cc-tile-metrics component unchanged and remove the empty state from cc-tile-requests.

The main reason is that the cc-tile-metrics requires some logic to determine whether the state is empty or not and we prefer leaving it to the smart while cc-tile-requests is more straightforward so the logic may remain within the component itself.

github-actions[bot] commented 3 weeks ago

🔎 The preview has been automatically deleted.

github-actions[bot] commented 3 weeks ago

🔎 A preview has been automatically published : https://clever-components-preview.cellar-c2.services.clever-cloud.com/cc-tile-requests/state-migration/index.html.

This preview will be deleted once this PR is closed.