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

Fix promql-query Vue component #516

Merged
merged 1 commit into from
Jun 4, 2024
Merged

Fix promql-query Vue component #516

merged 1 commit into from
Jun 4, 2024

Conversation

vincent-olivert-riera
Copy link
Contributor

This component had some issues that prevented it to render properly.

First, the unnecessary usage of 'this_ = this'. In a Vue component, 'this' always refers to the component itself.

Second, the unreliable way to change the display CSS property by accessing 'this_.$el.style.display'. In Vue, we can do conditional rendering using the v-show directive. We have defined a new 'ready' variable that becomes 'true' when the component is ready to be rendered, and use v-show to toggle the visibility of the component based on the value of that variable.

Third, the position of the '' in the template. This slot is meant to allow the user to describe the result of the query. For instance, if the result of the query is the number of samples, a good content of that slot would be the string 'Samples:', in order to display (for instance) 'Samples: 238' on the screen. However, since the slot was located at the right-hand side of the query result, this would have been displayed instead: '238 Samples:'.

Finally, we have simplified the logic of the 'fetch' call to remove duplicity and reduce the number of lines.


Before:

image

After:

image

This component had some issues that prevented it to render properly.

First, the unnecessary usage of 'this_ = this'. In a Vue component, 'this'
always refers to the component itself.

Second, the unreliable way to change the display CSS property by accessing
'this_.$el.style.display'. In Vue, we can do conditional rendering using the
v-show directive. We have defined a new 'ready' variable that becomes 'true'
when the component is ready to be rendered, and use v-show to toggle the
visibility of the component based on the value of that variable.

Third, the position of the '<slot></slot>' in the template. This slot is meant
to allow the user to describe the result of the query. For instance, if the
result of the query is the number of samples, a good content of that slot would
be the string 'Samples:', in order to display (for instance) 'Samples: 238' on
the screen. However, since the slot was located at the right-hand side of the
query result, this would have been displayed instead: '238 Samples:'.

Finally, we have simplified the logic of the 'fetch' call to remove duplicity
and reduce the number of lines.
@vincent-olivert-riera vincent-olivert-riera requested a review from a team as a code owner June 3, 2024 08:45
Copy link
Collaborator

@kfdm kfdm left a comment

Choose a reason for hiding this comment

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

LGTM

@kfdm kfdm merged commit d07aa31 into master Jun 4, 2024
5 checks passed
@kfdm kfdm deleted the fix-promql-query-component branch June 4, 2024 07:35
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