-
Notifications
You must be signed in to change notification settings - Fork 27
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
base: develop
Are you sure you want to change the base?
Redesign of homepage #845
Conversation
There was a problem hiding this 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
There was a problem hiding this comment.
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).
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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).
<script lang="ts"> | ||
export default { | ||
name: "LightLogo", | ||
}; | ||
</script> |
There was a problem hiding this comment.
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.
<script lang="ts"> | |
export default { | |
name: "LightLogo", | |
}; | |
</script> |
|
||
<script lang="ts"> | ||
export default { | ||
name: "DiscoLogoBoule", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
name derived from filename
name: "DiscoLogoBoule", |
customClass: { | ||
default: "bi bi-info-circle w-7 h-7", | ||
type: String, | ||
}, | ||
viewBox: { default: "-1 -1 18 18", type: String }, |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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" |
There was a problem hiding this comment.
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.
<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> |
There was a problem hiding this comment.
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? |
There was a problem hiding this comment.
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" /> --> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
dead comment
<!-- <Landing v-show="!started" @click="started = true" /> --> |
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:
Homepage after:
Other Changes:
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.