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

Redesign of homepage #845

Open
wants to merge 16 commits into
base: develop
Choose a base branch
from
Open

Conversation

iamsherlocked1891
Copy link
Collaborator

Changes to the Homepage:

1. Added a one-sentence description of DISCO:
A brief description of Disco has been included on the homepage.
2. Moved key information from the Info section:
Essential information previously located in the Info section has been moved to the homepage to improve visibility and accessibility.
3. Replaced GIF logo with SVG:
The DISCO logo in GIF format has been replaced with an SVG version.
4. Relocated the "Get Started" section:
The "Get Started" section has been moved to the bottom of the page to streamline the layout.

Hompage before:

before

Homepage after:

after_1

after_2

after_3

Other Changes:

  1. The "Info Section" icon has been removed from the sidebar
  2. On the About Page, laboratory logos have been updated and replaced with SVG versions.

Note on the MLO Logo:

There is a minor issue with the MLO logo: In the original SVG file, the laboratory name text is black, which works well for the light theme. However, when switching to a dark theme, the text should turn white for better visibility. Unfortunately, I was unable to achieve this result.

@iamsherlocked1891 iamsherlocked1891 marked this pull request as ready for review January 13, 2025 21:54
Copy link
Collaborator

@tharvik tharvik left a comment

Choose a reason for hiding this comment

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

thanks for the work, that makes a way more pro homepage, love it!

a few comments, mostly on style, aside from the text consistency on the homepage nothing blocking

while at it, you can also remove src/assets/{gif/DiscoGIF,svg/Disco,AboutUsIcon}.vue now that it is unused

Copy link
Collaborator

Choose a reason for hiding this comment

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

"Boule" is very français, using various languages in a codebase makes it harder for people not using every language used in it to work with. rename it to smth like "DiscoLogo" (as it is not only the ball).

Copy link
Collaborator

Choose a reason for hiding this comment

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

in dark mode, we can't read the subtitle, only a kinda omnious "optimization" shows up.

Copy link
Collaborator

Choose a reason for hiding this comment

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

ho right, I see that you added the fill color in code but it doesn't work when switching the theme after the mounting the component. that's why people are not reading the localStorage directly but are using a store to do that. we have a few around, for the tasks, the models, look into webapp/src/store and feel free to create your own for the user settings (which would only contain the theme for now).

Comment on lines +60 to +64
<script lang="ts">
export default {
name: "LightLogo",
};
</script>
Copy link
Collaborator

Choose a reason for hiding this comment

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

the name isn't different from the one derived from the file.

Suggested change
<script lang="ts">
export default {
name: "LightLogo",
};
</script>


<script lang="ts">
export default {
name: "DiscoLogoBoule",
Copy link
Collaborator

Choose a reason for hiding this comment

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

name derived from filename

Suggested change
name: "DiscoLogoBoule",

Comment on lines +68 to +72
customClass: {
default: "bi bi-info-circle w-7 h-7",
type: String,
},
viewBox: { default: "-1 -1 18 18", type: String },
Copy link
Collaborator

Choose a reason for hiding this comment

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

both seems unused, rm (and with the previous comment, it allows to remove the whole script block)

<!-- Information -->

<div
class="flex flex-col space-y-10 mt-8 text-justify py-3 sm:text-2xl md:text-2xl lg:text-2xl xlg:text-3xl"
Copy link
Collaborator

Choose a reason for hiding this comment

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

texts in here are rendered as black, better to use the text-slate-* classes to use the same color in the same context.

note: there is a bunch of incistencies in the UI, font color/size, gaps, margin, … a nicer way to ensure consistency would be to share CSS classes/var via webapp/src/assets/css/styles.css and add <style scoped> block to components which need it (and drop tailwindcss..). that would be much simpler for newcomers to help with styling; but yeah, much to do and not too much time, so I'm commenting where I can 🤷

<!-- Information -->

<div
class="flex flex-col space-y-10 mt-8 text-justify py-3 sm:text-2xl md:text-2xl lg:text-2xl xlg:text-3xl"
Copy link
Collaborator

Choose a reason for hiding this comment

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

consider using grid & grid-cols-1 xl:grid-cols-2 instead of flex. I find that makes it nicer on very large screen.

Comment on lines +58 to +110
<div class="flex flex-col space-y-10 p-6 lg:flex-row">
<RealTimeCollaboration class="w-full h-full lg:w-1/3 mx-auto" />
<p class="lg:w-1/2 text-justify text-2xl pr-4">
Disco is an easy-to-use mobile app and web software that enables
collaborative and privacy-preserving training of machine learning
models, running directly in your browser. Leveraging federated and
decentralized learning, Disco ensures seamless collaboration while
maintaining data privacy.
</p>
</div>

<div class="grid px-6 lg:grid-cols-2 gap-x-14 mx-2">
<div class="flex flex-col md:my-6">
<h6
class="font-bold text-justify leading-none text-xl tracking-wider sm:text-2xl md:text-3xl lg:text-3xl xlg:text-4xl"
>
Federated Learning
</h6>
<p
class="text-xl text-justify py-3 sm:text-2xl md:text-2xl lg:text-2xl xlg:text-3xl"
>
The key insight is to share weight updates instead of data - each user
trains on their own device and periodically shares weight updates with
a central server, while keeping data local at all times. The server
will agreggate all these weights between participants, and send them
back.
</p>
</div>
<FederatedGIF
class="bg-slate-100 rounded-lg border-slate-300 border-4 mt-4 lg:mt-0"
/>
</div>
<div class="grid px-6 lg:grid-cols-2 gap-x-14 mx-2 lg:my-8">
<DecentralizedGIF
class="bg-slate-100 rounded-lg border-slate-300 border-4 mt-4 lg:mt-0"
/>
<div class="flex flex-col md:my-6 lg:mt-0">
<h6
class="font-bold text-justify leading-none text-xl tracking-wider mt-3 sm:text-2xl md:text-3xl lg:text-3xl lg:text-right xlg:text-4xl"
>
Decentralized Learning
</h6>
<p
class="text-xl text-justify py-3 sm:text-2xl md:text-2xl lg:text-2xl xlg:text-3xl"
>
Building upon the same principles as in federated learning,
decentralized learning achieved allows collaboration and data privacy
without the need for a central coordinator. Updates are shared purely
via peer2peer communication. Disco puts users in control of the entire
collaborative training process, without a central point of failure.
</p>
</div>
</div>
Copy link
Collaborator

Choose a reason for hiding this comment

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

a bunch of theses uses custom padding, margin, gap-x and other "by hand layout" classes which I think can be greatly simplified by some well though mix of the top level div with gap-10 and consistent margins on every childs.
I recommend reading some documentation on flexbox and grid, that helped me understand a lot about layout in CSS
on firefox, you can also inspect the page and show the flex & grid constraints directly on the page (by clicking on the "flex" button next to the element), which is very useful to see why some elements are missbehaving.

<h6
class="font-bold mx-auto mb-8 text-justify leading-none text-xl tracking-wider sm:text-2xl md:text-3xl lg:text-3xl xlg:text-4xl"
>
Why use Disco?
Copy link
Collaborator

Choose a reason for hiding this comment

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

this title is describing a category, but it's the first time we see one looking like this (centered, without a body) (apart from the disco logo). consider either adding a title to the previous part, or make it "flow" more with the previous part.

</div>
</div>
<div class="flex flex-col justify-center">
<!-- <Landing v-show="!started" @click="started = true" /> -->
Copy link
Collaborator

Choose a reason for hiding this comment

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

dead comment

Suggested change
<!-- <Landing v-show="!started" @click="started = true" /> -->

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