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.

Sign up for GitHub

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

Jump to bottom

EPMRPP-82207 || Add events for Documentation links in Project Settings #3635

Merged
merged 3 commits into from
Nov 10, 2023
Merged

EPMRPP-82207 || Add events for Documentation links in Project Settings #3635

merged 3 commits into from
Nov 10, 2023

Conversation

Bam6ycha
Copy link
Contributor

@Bam6ycha Bam6ycha commented Nov 3, 2023

No description provided.

@Bam6ycha Bam6ycha requested a review from BlazarQSO November 3, 2023 08:03
@codecov-commenter
Copy link

codecov-commenter commented Nov 3, 2023 •
edited
Loading

Codecov Report

Merging #3635 (7c56dea) into develop (6485432) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff            @@
##           develop    #3635   +/-   ##
========================================
  Coverage    60.21%   60.21%           
========================================
  Files           79       79           
  Lines          857      857           
  Branches       124      124           
========================================
  Hits           516      516           
  Misses         314      314           
  Partials        27       27

📣 Codecov offers a browser extension for seamless coverage viewing on GitHub. Try it in Chrome or Firefox today!

@Bam6ycha Bam6ycha force-pushed the feature/EPMRPP-82207-Add-events-for-Documentation-links-in-Project-Settings branch from 8e08a8e to 0e584a2 Compare November 9, 2023 09:54
AmsterGet
AmsterGet requested changes Nov 9, 2023
View reviewed changes
Copy link
Member

@AmsterGet AmsterGet left a comment •
edited
Loading

Choose a reason for hiding this comment

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

I see that this part repeated at least 7 times.
image
Also I don't see much cases to reuse handleDescriptionClick and handleDescriptionKeyDown with the Layout component, so it can be encapsulated somewhere.
What do you think about creating a separate component that will display a formatted description and handle all the necessary events internally? In this case, the Layout description prop can be a react node.

app/src/components/main/analytics/events/ga4Events/projectSettingsPageEvents.js Outdated Show resolved Hide resolved
@Bam6ycha Bam6ycha force-pushed the feature/EPMRPP-82207-Add-events-for-Documentation-links-in-Project-Settings branch from 0e584a2 to bfa9742 Compare November 10, 2023 07:44
@Bam6ycha Bam6ycha requested a review from AmsterGet November 10, 2023 07:50
@Bam6ycha Bam6ycha force-pushed the feature/EPMRPP-82207-Add-events-for-Documentation-links-in-Project-Settings branch from bfa9742 to 7ea965e Compare November 10, 2023 08:07
@Bam6ycha Bam6ycha force-pushed the feature/EPMRPP-82207-Add-events-for-Documentation-links-in-Project-Settings branch from 7ea965e to 7c56dea Compare November 10, 2023 10:57
@Bam6ycha Bam6ycha requested a review from AmsterGet November 10, 2023 10:57
@sonarqubecloud SonarQubeCloud
Copy link

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@AmsterGet AmsterGet merged commit 261bffa into develop Nov 10, 2023
7 checks passed
@AmsterGet AmsterGet deleted the feature/EPMRPP-82207-Add-events-for-Documentation-links-in-Project-Settings branch November 10, 2023 11:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AmsterGet AmsterGet AmsterGet approved these changes

@Vadim73i Vadim73i Awaiting requested review from Vadim73i

@BlazarQSO BlazarQSO Awaiting requested review from BlazarQSO BlazarQSO is a code owner

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Bam6ycha @codecov-commenter @AmsterGet