Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

non standalone actions on custom controls #3395

Open
3 of 8 tasks
pjmuller opened this issue Nov 7, 2024 · 5 comments
Open
3 of 8 tasks

non standalone actions on custom controls #3395

pjmuller opened this issue Nov 7, 2024 · 5 comments

Comments

@pjmuller
Copy link

pjmuller commented Nov 7, 2024

Describe the bug

related to https://github.com/avo-hq/avo-advanced/pull/50

see https://www.loom.com/share/ee833adc463442ffb462435b77506b3d that demonstrates the problems below

  1. icon + text not grayed out while disabled (so not a clear signal to the user what is available/unavailable)
  2. when no text present seems to still open standalone=false when no records are selected
  3. inconsistent behavior, sometimes (but not always) when unchecking the checkbox, we are able to open the action

feature / code snippet request

  • could you explain how we can have a tooltip on the disabled actions saying they should first select an item, this will make it more foolproof

FYI: I also ran the same experiments having chrome developer console open + disable cache, so I should have all latest assets

module Avo
  module Resources
    class Tenant < Avo::BaseResource
      self.index_controls = -> do
        action Avo::Actions::Dummy, icon: 'heroicons/outline/envelope', style: :outline, title: 'Stuur Mailsjabloon', label: ''
      end
end

module Avo
  module Actions
    class Dummy < Avo::BaseAction
      self.standalone = false
      self.name = 'Dummy'
      # ...
    end
  end
end

System configuration

Avo version: 3.14

Rails version: 7.2

Are you using Avo monkey patches, overriding views or view components?

  • Yes. If so, please post code samples.
  • No

Impact

  • High impact (It makes my app un-usable.)
  • Medium impact (I'm annoyed, but I'll live.)
  • Low impact (It's really a tiny thing that I could live with.)

Urgency

  • High urgency (I can't continue development without it.)
  • Medium urgency (I found a workaround, but I'd love to have it fixed.)
  • Low urgency (It can wait. I just wanted you to know about it.)
Copy link
Contributor

This issue has been marked as stale because there was no activity for the past 15 days.

@github-actions github-actions bot added the Stale label Nov 24, 2024
@Paul-Bob
Copy link
Contributor

Hi @pjmuller thanks for reporting this.

Could you please verify if point 2. and 3. still an issue on the latest version?

Regarding point 1. I thought about it during the development and since is a custom cntrok where you can set the color even to purple if you want, feels strange to force a default behavior like changing text to black and gray on activation and deactivation.

Maybe some option like active_classes is the way, where you could configure classes to add on activation, same classes would be removed on deactivation... I've done some experimental development on this approach and it seems to work

@github-actions github-actions bot removed the Stale label Nov 25, 2024
@Paul-Bob
Copy link
Contributor

@pjmuller
Copy link
Author

pjmuller commented Nov 30, 2024

Hi @Paul-Bob ,

2 and 3 seems to be fixed (I went from 3.14.0 to 3.14.5). Thanks a lot!
regarding the styling point 1, yes makes sense to add specific disabled classes. Would be a great addition to make the UI more foolproof as now the users will start to "rage click" on the buttons as they look exactly the same as the standard ones.

Would be great to have this in 3.15 (or a minor release afterwards)

@Paul-Bob
Copy link
Contributor

Paul-Bob commented Dec 2, 2024

I've done some experimental development on this approach and it seems to work

This approach was effective for specific classes, but conflicts occasionally happens between custom classes and the default ones


regarding the styling point 1, yes makes sense to add specific disabled classes.

After discussing this with Adrian, we opted for other strategy. Each button style have a version with different opacity and we'll use that to differentiate when the button is active or disabled.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: No status
Development

No branches or pull requests

2 participants