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

SLI-1788 SLI-1784 SLI-1796 Create Walkthrough for SonarQube for IDE #1280

Merged

Conversation

eray-felek-sonarsource
Copy link
Contributor

@eray-felek-sonarsource eray-felek-sonarsource commented Jan 7, 2025

4 Page walkthrough the let user get familiar with the SonarQube IDE functionalities

@eray-felek-sonarsource eray-felek-sonarsource changed the title Feature/ef/sli 1784 SLI-1788 SLI-1784 SLI-1796 Create Walkthrough for SonarQube for IDE Jan 9, 2025
Copy link

Choose a reason for hiding this comment

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

The SonarLintWalkthroughToolWindow file is huge and very hard to follow, and I feel like has lots of duplications. I would suggest to either deduplicate common logic and object creations by extracting them to methods, or separate each walkthrough step in a separate file.

I also added some suggestions about wording and capitalization.

SONARQUBE_FOR_IDE + " supports the analysis of 15+ languages including Python, Java, Javascript, IaC domains along with secrets " +
"detection. " +
"<a href=\"" + RULE_SECTION_LINK + "\">Learn more</a>.<br><br>" +
"Detect issues while you code in an open files or run the analysis on more file in the <a href=\"#reportView\">report view</a>" +

Choose a reason for hiding this comment

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

Suggested change
"Detect issues while you code in an open files or run the analysis on more file in the <a href=\"#reportView\">report view</a>" +
"Detect issues in open files while you code, or run the analysis of more files from the <a href=\"#reportView\">Report view</a>" +

"<a href=\"" + RULE_SECTION_LINK + "\">Learn more</a>.<br><br>" +
"Detect issues while you code in an open files or run the analysis on more file in the <a href=\"#reportView\">report view</a>" +
".<br><br>" +
"Open a file and start your clean code journey.</body></html>");

Choose a reason for hiding this comment

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

Suggested change
"Open a file and start your clean code journey.</body></html>");
"Open a file and start your Clean Code journey.</body></html>");

"Apply the same set of rules as your team by using " + SONARQUBE_FOR_IDE + " in Connected Mode with SonarQube Cloud or SonarQube " +
"Server" +
".<br><br>" +
"With connected mode, benefit from advanced analysis like <a href=\"#taintVulnerabilities\">Taint Vulnerabilities</a> and open " +

Choose a reason for hiding this comment

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

Suggested change
"With connected mode, benefit from advanced analysis like <a href=\"#taintVulnerabilities\">Taint Vulnerabilities</a> and open " +
"With Connected Mode, benefit from advanced analysis like <a href=\"#taintVulnerabilities\">Taint Vulnerabilities</a> and open " +

Copy link
Member

@thahnen thahnen left a comment

Choose a reason for hiding this comment

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

I second Sophio here that this needs some de-duplication and documentation as well.

Copy link

Quality Gate failed Quality Gate failed

Failed conditions
4 New issues

See analysis details on SonarQube

Catch issues before they fail your Quality Gate with our IDE extension SonarQube IDE SonarQube IDE

Copy link
Member

@nquinquenel nquinquenel left a comment

Choose a reason for hiding this comment

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

It has been a long-standing practice to create new classes in Kotlin rather than Java. I'd recommend doing so, as it would help increase the code's readability (Swing is quite heavy to write).

I'd also move every related page to a dedicated directory, as the UI package is already quite packed.

Other than that, there are a few more comments (most of them are applicable to all the **Page classes), but the result is already quite visible and working, it's almost good!

var font = UIUtil.getLabelFont();
learnAsYouCodePagePanel = new JPanel(new BorderLayout());

var icon = new ImageIcon(Objects.requireNonNull(getClass().getResource("/images/sonarqube-for-ide-mark.png")));
Copy link
Member

Choose a reason for hiding this comment

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

Same here, get the image icon from SonarLintIcon. Also I'd recommend using SVG as much as possible

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When the new images arrive I will convert if there are not in svg

}

private static JEditorPane createWelcomePageText(Font font, Project project) {
var descriptionPane = new JEditorPane(SonarLintWalkthroughToolWindow.EDITOR_PANE_TYPE,
Copy link
Member

Choose a reason for hiding this comment

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

It may be personal, but I'm not a big fan of using huge blocks of HTML in Java Swing components. I'd rather have a clean layout and structure with proper layout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I used SwingHelper.createHtmlViewer as discussed


descriptionPane.addHyperlinkListener(new HyperlinkAdapter() {
@Override
protected void hyperlinkActivated(HyperlinkEvent e) {
Copy link
Member

Choose a reason for hiding this comment

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

I find this link management a bit dirty, I think breaking down the big HTML block into Swing components would help with this

Copy link
Contributor Author

Choose a reason for hiding this comment

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

As discussed I used SwingHelper.createHtmlViewer

src/main/java/org/sonarlint/intellij/ui/WelcomePage.java Outdated Show resolved Hide resolved
Copy link
Member

@nquinquenel nquinquenel left a comment

Choose a reason for hiding this comment

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

Another round of comments following the kotlin refactoring, it's looking better!

@eray-felek-sonarsource eray-felek-sonarsource force-pushed the feature/ef/SLI-1784 branch 4 times, most recently from 60b1159 to 37ae04a Compare January 21, 2025 15:15
@eray-felek-sonarsource eray-felek-sonarsource force-pushed the feature/ef/SLI-1784 branch 2 times, most recently from 4e9449f to 535e78b Compare January 21, 2025 17:02
@eray-felek-sonarsource eray-felek-sonarsource changed the base branch from master to feature/ef/intellij-walkthrough January 22, 2025 08:33
Copy link
Member

@nquinquenel nquinquenel left a comment

Choose a reason for hiding this comment

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

Let's merge it in the feature branch for now

Copy link
Member

@thahnen thahnen left a comment

Choose a reason for hiding this comment

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

LGTM, please fix these two typos.

Copy link
Member

@thahnen thahnen left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

@eray-felek-sonarsource eray-felek-sonarsource merged commit 514c750 into feature/ef/intellij-walkthrough Jan 22, 2025
30 checks passed
@eray-felek-sonarsource eray-felek-sonarsource deleted the feature/ef/SLI-1784 branch January 22, 2025 11:28
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.

4 participants