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

Completed remaning expor features #346

Open
wants to merge 8 commits into
base: main
Choose a base branch
from
Open

Completed remaning expor features #346

wants to merge 8 commits into from

Conversation

pierpuccini
Copy link
Contributor

@pierpuccini
Copy link
Contributor Author

pierpuccini commented May 6, 2020

This PR includes

  • Mailing lists
  • Publishers
  • Event detectors
  • Users lists
  • Watchlists

@terrypacker
Copy link
Contributor

terrypacker commented May 7, 2020

@pierpuccini this may be an issue for the other Lists but I just noticed that if I have 0 mailing lists and and 1 the export button does not show up at the top of the list header. Also if I refresh the page and delete delete it the export button is still there. These buttons should be dynamic based on number of items in the list 0 not there 1 or more button is there.

@terrypacker
Copy link
Contributor

@pierpuccini the watchlist icon should say 'Export' not 'Export all'.

@terrypacker
Copy link
Contributor

@pierpuccini I was not able to fully test the publishers because of this bug #348

@terrypacker
Copy link
Contributor

@pierpuccini not sure what the plan is for the User export but it currently does not use the filter but is labeled 'Export' not 'Export all' which is what it will do

@pierpuccini
Copy link
Contributor Author

@terrypacker regarding the user export, I left it like that because the plan is to use the filter, should I go back and correct that until the filter ability is added? Regarding the other issue mentioned I'll go back and revise.

@terrypacker
Copy link
Contributor

@pierpuccini for the user export that is fine to leave it as is for now.

@jazdw
Copy link
Contributor

jazdw commented May 7, 2020

@terrypacker

These buttons should be dynamic based on number of items in the list 0 not there 1 or more button is there.

I disagree. I think we are better off leaving the button displayed, I don't think hiding it really benefits the user experience at all. It can be a bit of a pain to determine how many items are in the list outside of the component. Therefore I think we should just not bother.

Hiding the button can also affect the flow of the page, if anything we should disable it. I think leaving it enabled also gives the user the option to see the structure of the json even if it is empty.

@terrypacker
Copy link
Contributor

@jazdw fine by me.

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.

3 participants