Skip to content

Javadoc issues #134

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Javadoc issues #134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e6f8a62
docs: fix javadoc
mlopezFC Dec 20, 2024
56d2091
build: enable javadoc failOnWarnings
mlopezFC Dec 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion pom.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -294,7 +294,7 @@
<configuration>
<quiet>true</quiet>
<doclint>none</doclint>
<failOnWarnings>false</failOnWarnings>
<failOnWarnings>true</failOnWarnings>
<links>
<link>https://javadoc.io/doc/com.vaadin/vaadin-platform-javadoc/${vaadin.version}</link>
</links>
Expand Down
6 changes: 3 additions & 3 deletions src/main/java/com/flowingcode/addons/applayout/AppLayout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,9 +203,9 @@ public void setDrawerBelowHeader(boolean drawerBelowHeader) {
}

/**
* Sets the drawerBelowHeader attribute so the drawer will be show below
* the header of the applayout
* @param drawerBelowHeader
* Sets the drawerRightAlignment attribute so the drawer will be shown
* with right alignment
* @param drawerRightAlignment
*/
public void setDrawerRightAlignment(boolean drawerRightAlignment) {
getElement().setAttribute("drawerAlign", drawerRightAlignment?"right":"left");
Comment on lines +206 to 211
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 20, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

⚠️ Potential issue

Documentation is accurate but there's a method duplication issue.

The documentation for setDrawerRightAlignment is clear and accurately describes the method's purpose. However, there appears to be a duplicate method setDrawerBelowHeader that still exists with different behavior.

Consider removing the setDrawerBelowHeader method if it's being replaced by setDrawerRightAlignment, or clarify the relationship between these methods if both behaviors are intended to coexist.

Copy link
Member

Choose a reason for hiding this comment

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

Comment must be addressed (https://github.com/FlowingCode/TechnicalDocs/blob/master/addons/pr-process.md)

Copy link
Contributor

Choose a reason for hiding this comment

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

@javier-godoy,

Thanks for pointing this out. From the script output, we can see that the method setDrawerBelowHeader is indeed used in test classes (DemoSettings and ApplayoutDemoView), so removing it completely would require aligning those references or clarifying the difference in intended behavior between “drawer below header” and “drawer right alignment.” If the plan is to replace setDrawerBelowHeader entirely with setDrawerRightAlignment, we should deprecate or remove setDrawerBelowHeader and update the test code accordingly. Alternatively, if both methods are still needed for different scenarios, we should clarify in the Javadoc why these two methods coexist and what each one does distinctly.

Let me know how you prefer to proceed:
• Remove setDrawerBelowHeader and references from the test classes, or
• Retain and clarify both methods’ purposes if they serve separate use cases.

I’m happy to help with either approach!

🔗 Analysis chain
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Let's search the codebase for references to setDrawerBelowHeader
rg 'setDrawerBelowHeader' -A 5 -B 5

# Then, let's search for references to setDrawerRightAlignment
rg 'setDrawerRightAlignment' -A 5 -B 5

Length of output: 10583

Expand Down
Loading