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

code(custom tooltip): enabling custom tooltips for relevant field types #90

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ansh-Sarkar
Copy link

Fixes: #16 by @pamfilos

Description: Introduces the ability for users to define tooltips for relevant fieldTypes. The solution has been ported from the original repository cernanalysispreservation/analysispreservation.cern.ch (#2800). The enhancement improves the user experience by providing informative tooltips that can be customized for each field, ensuring better clarity and guidance.

image

…m tooltip for relevant fieldtypes

Signed-off-by: Ansh Sarkar <anshsark18@gmail.com>
@Ansh-Sarkar
Copy link
Author

@pamfilos requesting you to kindly review this PR whenever convenient. Kindly do let me know if there are any changes required. Thank you.

@Ansh-Sarkar
Copy link
Author

Hi @pamfilos ! Looks like one of the tests is failing. Any suggestions on how to proceed from here ?

Copy link
Collaborator

@miguelgrc miguelgrc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @Ansh-Sarkar, thanks for your PR and sorry for the very late review. I have left some small comments.

Also, has the commitizen cli popped up when creating your commit? From your commit message it seems like the hook might not have triggered. Your commit message should look like feat(form): ... and be under 100 characters. If you check the commitlint check/test you can see it's failing for these reasons.

@@ -515,6 +522,7 @@ const simple = {
{ const: "number", title: "Float" },
],
},
tooltip: extra.optionsSchema.tooltip,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please move the tooltip up, here and in other fields, so that it appears directly under title and description (i.e. directly after ...common.optionsSchema)

@@ -640,7 +648,6 @@ const simple = {
type: {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add tooltip support also in switch

@miguelgrc
Copy link
Collaborator

Also, do you think you can update this test to verify that the tooltip is showing up when set? If you are not familiar with cypress I can take care of it.

@pamfilos
Copy link
Collaborator

Please update commit message with below fixes:
✖ header must not be longer than 100 characters, current length is 102 [header-max-length]
✖ type must be one of [build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test] [type-enum] - you have (custom-tooltip)

@Ansh-Sarkar
Copy link
Author

Ansh-Sarkar commented Nov 12, 2024

Also, do you think you can update this test to verify that the tooltip is showing up when set? If you are not familiar with cypress I can take care of it.

Yeah sure I'll get this done as well. Am sorry about the late response. Was not keeping well for a while. Am doing better now. Fixing this PR. Thanks!

@Ansh-Sarkar
Copy link
Author

Please update commit message with below fixes:
✖ header must not be longer than 100 characters, current length is 102 [header-max-length]
✖ type must be one of [build, chore, ci, docs, feat, fix, perf, refactor, revert, style, test] [type-enum] - you have (custom-tooltip)

Sure. Thanks.

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