Possibilité de catégoriser / grouper les scènes

[Archive] Feature request
, ,
61

@pierre-gilles : I just fixed all the feedback

1 Like
62

Thanks for the feedback, it’s much better!

I have some UX feedback and a technical one, nothing too serious :slight_smile:

We’re on the right track, I think this can be merged soon!

1 Like
63

I’ve just corrected your latest feedback :slight_smile:

64

Thanks for the feedback :slight_smile:

I still have a few review comments on small details :slight_smile: : https://github.com/GladysAssistant/Gladys/pull/1900#pullrequestreview-1726960919

65

Thanks for the review :slight_smile:

I’ve made the changes except for the « deselect all » button. I’m not sure that’s useful. We’ll see depending on how everyone uses it. What do you think?

66

It all depends on the number of tags added.

67

Ok, I’m happy to see it in real use, but in my opinion the request will come quickly! When I tested I found it inconvenient

1 Like
68

It’s all good for me @Lokkye, there’s just ESLint yelling on your last push :stuck_out_tongue:

And I just have one question about the Cypress tests:

After these two little things, it’s good for me to merge! Can’t wait to publish this :slight_smile:

Edit: Ah wait, small regression on the « New scene » view:

Before:

Screenshot 2023-11-16 at 10.10.55
Screenshot 2023-11-16 at 10.10.551150×1100 33.2 KB

After:

Screenshot 2023-11-16 at 10.10.49
Screenshot 2023-11-16 at 10.10.491046×1036 32.6 KB

It looked cleaner before, didn’t it?

69

Yes it’s better with the button at the bottom

70

It doesn’t visually bother me, but the button at the bottom makes much more sense.
The flow is Name > Icon > Confirm.

71

@pierre-gilles @Hizo @lmilcent : Yes I agree with you. I still wonder how I managed to swap those two components when I wasn’t supposed to touch that code :slight_smile:

@pierre-gilles : I just made the fixes on the PR and rebased onto master so you can easily merge it

72

It’s good for me @Lokkye, thanks for the fixes :clap:

It’s merged into master and will be included in the next Gladys release :slight_smile:

I just have a few small notes that weren’t blocking for the review that I noticed:

  • With Preact, there’s no need to rename class= to className=. className is a React-specific thing that doesn’t apply to Preact; it’s even recommended to stick with class= :slight_smile: (but className still works)
  • To hide content on small screens, you can use built-in Bootstrap classes, which avoids re-writing CSS ( Display property · Bootstrap )
2 Likes
73

This feature is available in Gladys Assistant 4.31! :rocket:

I’m closing this thread to free up the votes — feel free to create a new thread if you encounter any bugs with this feature :slight_smile:

2 Likes