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

Support for storing expressions #52

Closed
wants to merge 2 commits into from

Conversation

rickardkarlsson
Copy link

@rickardkarlsson rickardkarlsson commented Mar 20, 2021

I was struggling with saved expressions in a document, and copy/pasted them into expressionbox all the time.
With this pull request, I add support for saving/loading/removal expressions.

@rickardkarlsson rickardkarlsson changed the title Support for managing saved expressions Support for storing expressions Mar 21, 2021
@0xbs
Copy link
Owner

0xbs commented Mar 25, 2021

Thank you for your contribution! I finally found time to have a look at it and have some remarks:

The first thing I noticed was that the PGF window is now larger than the dungeon finder window which does not look as good as before. If we want to go this way, we have to find a way that keeps the aesthetics.

When trying it out, my initial expectation was that it saves all filters, but it saves only the advanced expression. I assume other players expect the same and I would very much prefer saving everything. Also the load button seems unnecessary as clicking on the dropdown entry automatically loads it.

To be honest, I would very much prefer something like the tabs on the right like what has been done on the Rematch addon. A player named Enkol has done something like this in the past (see branch feature/tabs), but it does not work properly with the current version (e.g. icon selection) and I did not like some aspects of it:

  • It removes the PGF checkbox
  • It hides the PGF window initially which makes it harder to see and use the addon
  • No tooltips on tabs except + tab
  • It could be a integrated better (although I kind of liked the aspect that I could throw away two files and everything was like before)
  • Usage of global variables from the icon selection should be removed

Maybe you could have a look at it and try to revive it together with your code?

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

Successfully merging this pull request may close these issues.

2 participants