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

feat: Initial web app for the court #69

Merged
merged 88 commits into from
Jun 9, 2022
Merged

feat: Initial web app for the court #69

merged 88 commits into from
Jun 9, 2022

Conversation

jaybuidl
Copy link
Member

@jaybuidl jaybuidl commented Apr 6, 2022

No description provided.

@jaybuidl jaybuidl requested a review from alcercu April 7, 2022 16:07
@jaybuidl jaybuidl marked this pull request as draft April 7, 2022 16:07
@jaybuidl jaybuidl requested a review from AndreiMVP April 7, 2022 17:14
@jaybuidl jaybuidl marked this pull request as ready for review April 7, 2022 17:28
@jaybuidl jaybuidl marked this pull request as draft April 7, 2022 21:07
@AndreiMVP
Copy link
Contributor

I think we should have the file organisation as simple as is convenient. If a component is used multiple times by all means it should be in a separate file. But in case of a component with <50 lines which will be used only once I don't see why it should be in a separate file. It only makes the directory tree more complex and navigating between components harder. This applies to the header and footer related components. I would even say that the layout, header and footer components should be one file.

web/src/index.tsx Outdated Show resolved Hide resolved
@alcercu
Copy link
Contributor

alcercu commented Apr 8, 2022

Thanks for the refactor @AndreiMVP! Regarding the file organization, I prefer to keep it in separated files even if these components are used once and only in the context of the layout. This is mainly because they might be small now, but once we start adding the desktop styling they might grow. Also, I think it's better to keep them abstracted, so in the future, if someone wants to modify some part of the layout, not related to the header and footer, they can focus directly on the part that matters.

@jaybuidl jaybuidl linked an issue Jun 3, 2022 that may be closed by this pull request
@jaybuidl jaybuidl added this to the prealpha-3 milestone Jun 3, 2022
@jaybuidl jaybuidl changed the title feat: web app for the court feat: Initial web app for the court Jun 3, 2022
@jaybuidl
Copy link
Member Author

jaybuidl commented Jun 3, 2022

Let's try merge this branch and continue working in a new more specific one, maybe feat/cases-page.

AndreiMVP
AndreiMVP previously approved these changes Jun 4, 2022
Copy link
Contributor

@AndreiMVP AndreiMVP left a comment

Choose a reason for hiding this comment

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

Looks OK for now

@alcercu
Copy link
Contributor

alcercu commented Jun 6, 2022

@jaybuidl @AndreiMVP I think this is ready to be merged, the last codeclimate duplication issues are in the theme file and in a file (LatestCases) that it's not fetching data yet (so the duplication comes from the manually-inserted data). There are issues regarding some type exporting, I think this is due to the difference between tslint (deprecated) and eslint with typescript support (the one we use locally), we need to look into updating codeclimate's linter.

@alcercu alcercu marked this pull request as ready for review June 6, 2022 14:58
@codeclimate
Copy link

codeclimate bot commented Jun 8, 2022

Code Climate has analyzed commit d145e28 and detected 123 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 3
Duplication 32
Style 88

View more on Code Climate.

@sonarqubecloud
Copy link

sonarqubecloud bot commented Jun 8, 2022

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
1.8% 1.8% Duplication

@alcercu alcercu merged commit e117efd into master Jun 9, 2022
@jaybuidl jaybuidl deleted the feat/web branch June 9, 2022 10:43
@jaybuidl jaybuidl restored the feat/web branch November 24, 2022 02:59
@jaybuidl jaybuidl deleted the feat/web branch November 24, 2022 03:11
Params10 pushed a commit that referenced this pull request Feb 3, 2023
Feat: Initial web app for the court
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Web: Home page
3 participants