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

Hide fields with empty answer #83

Closed
wants to merge 2 commits into from

Conversation

pplulee
Copy link

@pplulee pplulee commented Jan 16, 2024

related to issue #82

Changes proposed in this pull request:

The fields with the empty answer will be hidden on user profile page, rather than display the field name with an empty behind.

Reviewers should focus on:
Maybe this could be implemented in backend?

Confirmed

  • Frontend changes: tested on a local Flarum installation.
  • Backend changes: tests are green (run composer test).

Required changes:

pplulee and others added 2 commits January 16, 2024 14:47
Includes transpiled JS/TS, and Typescript declaration files (typings).

[skip ci]
@luceos
Copy link
Contributor

luceos commented Jan 17, 2024

I don't know if we want this honestly. It feels like enforcing one way of using the fields because you prefer it. If anything, it might make more sense to allow configuration that the field is hidden if no answer is given. Regardless someone else from @FriendsOfFlarum/maintainers might think otherwise and agree with you.

@pplulee
Copy link
Author

pplulee commented Jan 17, 2024

I don't know if we want this honestly. It feels like enforcing one way of using the fields because you prefer it. If anything, it might make more sense to allow configuration that the field is hidden if no answer is given. Regardless someone else from @FriendsOfFlarum/maintainers might think otherwise and agree with you.

Thanks for the feedback, I also think it would be better to have an option to control whether it shows or not. Because I just getting into flarum and don't have the ability to implement it on the backend now.

@luceos
Copy link
Contributor

luceos commented Jan 27, 2024

@imorland this one can probably be closed until a better pr is provided with the agreed upon solution, unless you think this would actually work for this extension.

@pplulee pplulee closed this Jan 28, 2024
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