Skip to content
This repository has been archived by the owner on Feb 4, 2020. It is now read-only.

Quickicon improvements #279

Merged
merged 4 commits into from Jun 6, 2019
Merged

Quickicon improvements #279

merged 4 commits into from Jun 6, 2019

Conversation

bembelimen
Copy link
Contributor

Use context for different plugin loading.

@brianteeman
Copy link
Contributor

I assume this is to allow 3pd quickicons to display? Will test tomorrow

@brianteeman
Copy link
Contributor

brianteeman commented Jun 6, 2019

I am guessing this is the intended display?

image

If so then I guess that works from a display perspective although I am not a fan of the title.

The problem comes with the Edit module screen for the module.

The logic is very confusing here for the user. Icons can be selected based on their group and in some cases additionally by the toggle.

image

@bembelimen
Copy link
Contributor Author

bembelimen commented Jun 6, 2019

This PR resolves the "bad concept style" of the current implementation with the plugin implementation.

The big padding is removed via: #280
The "problem" of the quickicons at all is the combination of "hard coded" icons directly from the module itself and the plugin triggers which allows plugins to laod their own icons. So the "group" is responsible for the context of the plugin trigger, the toggles for the hard coded icons.

The best solution would probably be: create a plugin for each icon. But that's out of scope for this PR.

This PR should just smoothing out code smells.

@chmst
Copy link
Contributor

chmst commented Jun 6, 2019

The code works fine for me. If it is merged, we can work on a better solution. I agree that this is confusing for users and also has to be improved concerning a11y.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants