search

Shared-Reality-Lab / IMAGE-browser

IMAGE project browser extensions & client-side code
Other
2 stars 0 forks source link

UI changes and functionality should be tied to feature flags, not to specific devices #357

Closed jeffbl closed 1 year ago

jeffbl commented 1 year ago

As currently implemented, features like whether to bring up new tabs vs. windows are tied specifically to whether it is running on mobile or desktop. For example, this line of code:

https://github.com/Shared-Reality-Lab/IMAGE-browser/pull/350/files#diff-11db236acd6b02b1229e7d325e9bf8edb521032e6388f68b6a884ec24bbe291fR475

This has two problems:

  1. If we want to change the behavior on a device, e.g., make it do tabs on the desktop, we have to go in and change many lines of code, everywhere this is checked.
  2. If we want to make it dynamic, e.g., by adding an option in the options page to let the user pick which they want, we can't do that since it is hard coded everywhere by device type.

Propose:

  1. Any such feature should be triggered with a specific flag for just that feature. In this case some sort of flag for new window type, which can be new tab or new window (and maybe in the future could be current page, in case we need that?)
  2. The defaults for each feature can be set by device type, as currently detected, but then this is done in only one place.

With that setup, the two problems listed above are resolved, and the code is much more flexible.

@21satvik Are there other such things we have effectively hard coded in this way? If so, work items should be logged for them as well.

jeffbl commented 1 year ago

@jaydeepsingh25 @21satvik This is resolved by #358, correct? If so, please link and close.

21satvik commented 1 year ago

@jeffbl This has already been resolved in #358 . So closing it now.